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

consistent "document" / "documents" fields for insert commands #141

Closed
amorton opened this issue Feb 16, 2023 · 12 comments · Fixed by #402
Closed

consistent "document" / "documents" fields for insert commands #141

amorton opened this issue Feb 16, 2023 · 12 comments · Fixed by #402
Assignees

Comments

@amorton
Copy link
Contributor

amorton commented Feb 16, 2023

Ensure we have a consistent and document use of "document" / "documents" for the insertOne and insertMany commands - and this matches what we use in the response document.

ensure same in:

  • server code
  • spec
  • driver code
  • postman
@ivansenic
Copy link
Contributor

@amorton I am not sure I follow the actual task here, currently in the implementation:

  • insertOne has a field document
  • insertMany has a field documents

Is this how we want it to be or not? If so then I only need to double check the spec, driver and the postman, right?

@amorton
Copy link
Contributor Author

amorton commented Apr 24, 2023

the original idea was to have them both be the same, which i guess would be "documents". And it would be the same for the find and findOneX calls as well so we always have the same document or documents.

I think there are two outcomes:

  1. It's the same everywhere, even if it does not make sense. i.e. insertOne uses documents.
  2. It's different, but correct where it makes sense. e.g. insertOne is document. findOne returns document. findMany returns documents.

Can you chat with the team in the USA including @kathirsvn and work out what is the best approach. The main thing is we are doing things deliberately.

@ivansenic
Copy link
Contributor

@kathirsvn Any chance we sync on this?

@kathirsvn
Copy link
Contributor

kathirsvn commented Apr 24, 2023 via email

@ivansenic
Copy link
Contributor

@amorton We agreed to go for the option #2. So we will change the response data.

  • In case there are multiple documents, we will return data.documents and data.nextPagingState
  • In case there is a single document (findOneXXX), we will only return data.document

Kathir said we need to publish this in a release first and then he will do the driver updates.

@maheshrajamani OK with you?

@maheshrajamani
Copy link
Contributor

@ivansenic That should be fine.

@ivansenic
Copy link
Contributor

@maheshrajamani @kathirsvn What are we doing with the delete and update commands, they are also returning data? should deleteOne and updateOne also have data.document?

@maheshrajamani
Copy link
Contributor

@ivansenic delete[One/Many] and update[One/Many] don't return any document in response. The document is returned for findOneAndUpdate, findOneAndReplace and findOneAndDelete. It will always be one document in response so has to be data.document.

@ivansenic
Copy link
Contributor

@ivansenic delete[One/Many] and update[One/Many] don't return any document in response. The document is returned for findOneAndUpdate, findOneAndReplace and findOneAndDelete. It will always be one document in response so has to be data.document.

@maheshrajamani ok but operation does:

https://github.com/stargate/jsonapi/blob/6cff90ffc739ce96b74ef9da744130b0e694a40b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DeleteOperationPage.java#L74-L84

https://github.com/stargate/jsonapi/blob/6cff90ffc739ce96b74ef9da744130b0e694a40b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/UpdateOperationPage.java#L65-L72

@maheshrajamani
Copy link
Contributor

boolean returnDocument flag will come in as false in case of delete[One/Many] and update[One/Many]

@ivansenic
Copy link
Contributor

boolean returnDocument flag will come in as false in case of delete[One/Many] and update[One/Many]

@maheshrajamani thanks.. so basically I can always assume that it's a single response in those delete and update op pages?

@maheshrajamani
Copy link
Contributor

@ivansenic Yes it will be a single document.

@sync-by-unito sync-by-unito bot changed the title consistent "document" / "documents" fields for insert commands consistent "document" / "documents" fields for insert commands Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants