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

Let clients define objectId using mongo/mongoose ObjectId() #6101

Closed
rhuanbarreto opened this issue Oct 2, 2019 · 12 comments
Closed

Let clients define objectId using mongo/mongoose ObjectId() #6101

rhuanbarreto opened this issue Oct 2, 2019 · 12 comments
Labels
type:feature New feature or improvement of existing feature

Comments

@rhuanbarreto
Copy link
Contributor

This issue follow up on #4170 but goes deeper in it.

It's not really safe to leave the user to set an objectId arbitrarily, but for offline PWAs it's almost mandatory that an ID must be generated offline. Normally UUIDs are used, but as Parse uses MongoDB, we can generate objectId's using the native mongo library for Node.js or Mongoose ObjectId Type for Client Javascript Applications.

So on Parse Server, this client-side created id could be allowed using a specific __type when you set this on a POST request, extending the normal JSON types. Also, on PostgreSQL you can allow the UUID __type, once it's more normal to use.

Also, the risk of overwriting objects using this method falls over the client/developer and not the server. So anyone that is using this "advanced" method will assume the risks of ID collision. Even UUIDs have this risk despite being negligible on small projects.

I think this is an important feature to be considered and it will make a big leap forward for using Parse Server on offline PWAs.

@davimacedo
Copy link
Member

Can you detail your use case? What is the situation of your PWA app that you would need to create an object id to be sent to the server later on?

@davimacedo davimacedo added type:feature New feature or improvement of existing feature discussion labels Oct 4, 2019
@AugustoAmaral
Copy link

I'm having the same problem, I have an application that needs to work both offline and online, so every object I create, I have to make it available for the user to use and store the request so it can be done as soon as the application comes online.

@davimacedo
Copy link
Member

But why do you need to create the id beforehand?

@rhuanbarreto
Copy link
Contributor Author

HI @davimacedo I will try to do my best.

Imagine you are in a React PWA application in offline state but with a Parse Server backend. You don't want your user to stop using your app meanwhile it's offline. For example a Sales/CRM application where the sellers are traveling in cars and internet connection is unstable.

So you can use Redux/Redux-Persist to store your full state locally and avoid interruption when you want to access data. But what about creating/updating/deleting data in an offline state? Then you can use an Optimistic UI to generate all the data locally without connecting to the server, updates the UI in an optimistic way and sync it back when it's online again. I personally use Redux-Offline for this connected to a Ruby on Rails backend that is connected to a PostgreSQL database. Redux offline creates a queue of fetch'es to be done and triggers commit or rollback actions based on the backend response. It's a really good library!

But once a record is created locally in the device, if I want to update or delete it, I can't, due to lack of unique identification (I can't update or delete what I cannot find and the create dispatch will run before other operations). So, what I do today is generating an UUIDv4 on client side, then the backend send this directly to the database. Of course, if the ID field is missing on the create request, the database generates this UUID for me and the backend gives a response with the record id.

Until now, Parse Server is facilitating a lot about my work on backend side, because all simple CRUD operations are already done and you need only to implement validations and ACLs. But this feature about accepting IDs from client side is the last thing needed to move to parse for any other projects we will do.

I could see that ID generation does not use the default MongoDB mechanism, but it's not clear also how can we generate this in the case that this feature could be supported.

Anything to clarify we can also make a call and discuss.

@rhuanbarreto
Copy link
Contributor Author

Just to add, this goes further than the local datastore on https://github.com/parse-community/docs/blob/gh-pages/_includes/js/local-datastore.md. Pinning objects still require the creation of the object on the backend before it is pinned. With ID creation on the client, you don't really need pinning. You can deal with localstorage/redux/persistence without any problems.

@davimacedo
Copy link
Member

@acinader @dplewis what do you guys think about this?

@dplewis
Copy link
Member

dplewis commented Oct 28, 2019

It's not really safe to leave the user to set an objectId arbitrarily

I agree with you but I don’t see where in the code a user can actually do this.

Pinning objects still require the creation of the object on the backend before it is pinned.

This isn’t true. You don’t need to create objects on the backend before you pin. All offline objects have a localid uuid that is used for handling data (as of 2.8.0).

I should update LDS docs.

@rhuanbarreto
Copy link
Contributor Author

I agree with you but I don’t see where in the code a user can actually do this.

@dplewis actually in Parse Server you can't set the user's objectId or id property. It gets an error on the following piece of code in src/RestWrite.js:

  if (!query && data.objectId) {	
    throw new Parse.Error(	
      Parse.Error.INVALID_KEY_NAME,	
      'objectId is an invalid field name.'	
    );	
  }	
  if (!query && data.id) {	
    throw new Parse.Error(	
      Parse.Error.INVALID_KEY_NAME,	
      'id is an invalid field name.'	
    );	
  }

But even if I disable this validation, the Id is overwritten.

This isn’t true. You don’t need to create objects on the backend before you pin. All offline objects have a localid uuid that is used for handling data (as of 2.8.0).

But how do you sync back the ID of the pinned object when you are online again?

@dplewis
Copy link
Member

dplewis commented Nov 1, 2019

@rhuanbarreto The LDS can use some improvement. I once tried to more syncing options besides await Parse.LocalDatastore.updateFromServer(). I wanted to have an open discussion about it but lack of feedback. You can read it here parse-community/Parse-SDK-JS#734

Also, the risk of overwriting objects using this method falls over the client/developer and not the server. So anyone that is using this "advanced" method will assume the risks of ID collision. Even UUIDs have this risk despite being negligible on small projects.

This among other unforeseen reason is why I personally am against this but I don't want to stop progress. I'd rather add saveEventually() from iOS and android to reduce the risk of overriding objects`.

If the client / developer understands the risk, I'm for it. Feel free to open a PR and we can move the dicussion there.

@acinader Thoughts?

@rhuanbarreto
Copy link
Contributor Author

@dplewis PR created. We can move the discussion. Just give me the guidance on where the ID generation happens and I can make all the changes.

@rhuanbarreto
Copy link
Contributor Author

Hi @dplewis @davimacedo, have a look on PR #6177 and share your thoughts! I think it's ok and stable now.

dplewis pushed a commit that referenced this issue Dec 17, 2019
* #6101 Let users define objectId

* Add `allowCustomObjectId` to PS Option

* Add checking in objectId creation

* Add test

* Update docs

* Update definition

* Change default to false

* throw on empty, null, undefined

* better tests

* unused async

* removed comment

* retain comment

* Linting fix according to contributing spec.
@dplewis
Copy link
Member

dplewis commented Dec 17, 2019

Closing via #6177

@dplewis dplewis closed this as completed Dec 17, 2019
UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this issue Mar 21, 2020
* parse-community#6101 Let users define objectId

* Add `allowCustomObjectId` to PS Option

* Add checking in objectId creation

* Add test

* Update docs

* Update definition

* Change default to false

* throw on empty, null, undefined

* better tests

* unused async

* removed comment

* retain comment

* Linting fix according to contributing spec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests

4 participants