Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow soft delete (that rely on Delete rather than Update permissions) #225

Open
apuigsech opened this issue Jan 3, 2019 · 15 comments
Open

Comments

@apuigsech
Copy link
Contributor

apuigsech commented Jan 3, 2019

I am going to explain my use case to see if you can help me find the best way to implement it using rest layer.

There are certain type of objects that I need to deprecate based on a TTL (time to live), so I created a field named 'ttl' on the schema with that configuration:

"ttl": {
    Default: 0,
    Validator: &schema.Integer{},
    Filterable: true,
},

Having 0 as 'ttl' means that the object is never deprecated. On the case it's not 0, the deprecation time must be updated + ttl.

Now I need to be able to sort and filter based on that deprecation time, but there isn't a specific field with that data, so i need to create it. The characteristics I need for that field are:

  • It must be ReadOnly, so nobody can write it directly but only through the 'ttl' value.
  • It must be Sortable and Filtrable.
  • If the 'ttl' is 0, It should have no value.

I cannot use OnInit or OnUpdate to set the value, because on the init and update function I don't have access to the value of the 'ttl' field, so I tried to use Hooks for OnInsert and OnUpdate; It works when creating the object, but not when updating it (because it's read only).

@smyrman
Copy link
Collaborator

smyrman commented Jan 3, 2019

Commenting although you are still editing the post.

I wold recommend solving the TTL on a database level based on your TTL field. For MongoDB, check out https://docs.mongodb.com/manual/core/index-ttl/

@apuigsech
Copy link
Contributor Author

The problem is that postgres (the db i am using) is not implementing any kind of ttl.

@smyrman
Copy link
Collaborator

smyrman commented Jan 6, 2019

Assuming a Nullable timestamp field in the DB named deleteAt in the schema, you cold have a goroutune that does a Clear with filter {deleteAt:{$lt: now} at a reasonable interval.

@smyrman
Copy link
Collaborator

smyrman commented Jan 6, 2019

Pseudo code for the filter...

@apuigsech
Copy link
Contributor Author

This is exactly the solution I am working on. The only problem is that I cannot meet the requirement of forbidding the direct edit of 'DeleteAt' (as I want it can be only modified through the 'ttl' field).

Maybe makes sense to create another attribute on which is "Computed". Computed fields are those that cannot be edited directly but they are computed on INSERT/UPDATE/REPLACE time.

Opinions?

@Dragomir-Ivanov
Copy link
Contributor

Is it really necessary to this to go through rest-layer? I have a use case to atomically increase an number in Mongo with $inc, so I will do it with direct DB connection, from rest-layer hook. Behind the hands of rest-layer,but I guess it will work, because rest-layer doesn't cache any DB data.

@apuigsech
Copy link
Contributor Author

I am not sure which is the best way to approach that, but I think that having Computed fields may be very powerful in some cases.

@Dragomir-Ivanov
Copy link
Contributor

Well, depending on what data they use to compute, I guess doing the computation in a hook is a way to go. I would prefer rest-layer to stay relatively simple, so it has lower entry barrier, but can be extended if needed for more complex scenarios. Hooks are one good extension point.

@apuigsech
Copy link
Contributor Author

I am using Hooks right now to implement what I want, and it works. But I want that nobody is able to edit the field directly through the REST interface :(

@Dragomir-Ivanov
Copy link
Contributor

Well, there are before and after hooks. So if you use before hook to catch any modification attempt, you may strip this field, or raise an error and block the REST call.

@smyrman
Copy link
Collaborator

smyrman commented Jan 8, 2019

Doesn't setting the field ReadOnly: true work? The resource hook can still update the field as it happens post schema validation.

@apuigsech
Copy link
Contributor Author

It doesn't work. It returns an error message because the field is ReadOnly and with the Hook I need to change it.

My hook is quite simple;

func (dh DeprecateHandler) OnInsert(ctx context.Context, items []*resource.Item) error {
	for _, i := range items {
		if i.Payload["ttl"].(int) > 0 {
			i.Payload["deleteAt"] = time.Now().Local().Add(time.Hour * time.Duration(i.Payload["ttl"].(int)))
		 }
	}	
	return nil
}




func (dh DeprecateHandler) OnUpdate(ctx context.Context, item *resource.Item, original *resource.Item) error {
	ttl_item, ok_item := item.Payload["ttl"].(int)
	ttl_original, ok_original := item.Payload["ttl"].(int)

	if ok_item && ttl_item > 0 {
		item.Payload["deleteAt"] = time.Now().Local().Add(time.Hour * time.Duration(ttl_item))
	} else {
		if ok_original &&  ttl_original > 0 {
			item.Payload["deleteAt"] = time.Now().Local().Add(time.Hour * time.Duration(ttl_original))
		}
	}

	return nil
}

@smyrman
Copy link
Collaborator

smyrman commented Jan 10, 2019

You are right 🤦‍♂️ My mistake.

There are some work-around for setting read-only fields but it's not available to the hook. It's used e.g. when setting fields from the path:

It wold be nice if there was an easier way to bypass Read-Only fields; we certainly have a use-case for it, e.g. when we are using RPC to update resources directly. But I don't have a good idea for how it cold be designed.

@smyrman
Copy link
Collaborator

smyrman commented Jan 10, 2019

Longer term, I plan to have a look at this here: https://github.com/searis/schema. However I cannot commit to any particular timeline for this now.

Probably nice to come up with a solution that works for the current schema package.

@smyrman
Copy link
Collaborator

smyrman commented Jan 10, 2019

Possible work-around for now:

If you always set the field in your hook on insert/update (or remove it from the payload if present when ttl is 0), any value the user may set will be overwritten so it doesn't hurt that the field is not read-only.

The only thing you won't get from this is the validation error when users are trying to set it. It's not ideal, but perhaps it's workable.

Possible fix in the schema package:

Here is one possible thought. We add a wrapper type that can be embedded in the payload programatically to skip validation.

package schema

type SkipValidation struct{
    Value interface{}
}

When a Field validation method encounters a value of type SkipValidation, it returns the value.(SkipValidation).Value instead of passing the value to the field.Validator.

So in this case:

func (dh DeprecateHandler) OnInsert(ctx context.Context, items []*resource.Item) error {
	for _, i := range items {
		if i.Payload["ttl"].(int) > 0 {
			i.Payload["deleteAt"] = schema.SkipValidation{
				Value: time.Now().Local().Add(time.Hour * time.Duration(i.Payload["ttl"].(int))),
			}
		 }
	}	
	return nil
}

Perhaps could even type SkipValidtion interface{} work, but that needs to be tested. Not sure how to resolve a "nested" interface{} reference in go.

@smyrman smyrman changed the title Implementing a "Deprecate" timestamp field Allow soft delete (that rely on Delete rather than Update permissions) Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants