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

single upsert and bulk Upsert accept divergent document types #22310

Closed
2 of 6 tasks
agorischek opened this issue Jun 20, 2022 · 6 comments
Closed
2 of 6 tasks

single upsert and bulk Upsert accept divergent document types #22310

agorischek opened this issue Jun 20, 2022 · 6 comments
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. Service Attention Workflow: This issue is responsible by Azure service team.

Comments

@agorischek
Copy link

  • Package Name: @azure/cosmos
  • Package Version: 3.16.1
  • Operating system: Windows ARM
  • nodejs
    • version: 16.15.1
  • browser
    • name/version:
  • typescript
    • version: 4.7.4
  • Is the bug related to documentation in

Describe the bug
The Upsert bulk operation accepts fewer input types than the upsert non-bulk method. The bulk approach requires a JSONObject, while the single upsert approach accepts unknown. This seems to prevent submitting documents that are instances of custom classes in bulk. Is this intentional?

To Reproduce

  1. Create a JavaScript object from a custom class
  2. Attempt to write it to Cosmos using Upsert operation via bulk method

Expected behavior
Object is written as document, just like as if upsert were called directly.

Additional context
Code sample:

  // Case 1: Plain object
  const document1 = { id: "1" };
  // TypeScript is fine with this and the operation succeeds
  await container.items.upsert(document1);

  // Case 2: Class
  class Document {
    constructor(public id: string) {}
  }
  const document2 = new Document("2");
  // TypeScript complains:
  //   Type 'Document' is not assignable to type 'JSONObject'.
  //     Index signature for type 'string' is missing in type 'Document'.ts(2322)
  await container.items.batch([
    {
      operationType: "Upsert",
      resourceBody: document2,
    },
  ]);

  // Case 3: Class with type casting
  const document3 = new Document("3");
  // Operation succeeds
  await container.items.bulk([
    {
      operationType: "Upsert",
      resourceBody: document3 as unknown as JSONObject,
    },
  ]);
@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Jun 20, 2022
@azure-sdk azure-sdk added Client This issue points to a problem in the data-plane of the library. Cosmos needs-team-triage Workflow: This issue needs the team to triage. labels Jun 20, 2022
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jun 20, 2022
@xirzec xirzec removed the needs-team-triage Workflow: This issue needs the team to triage. label Jun 21, 2022
@xirzec xirzec added this to the [2022] July milestone Jun 21, 2022
@sajeetharan sajeetharan moved this from Spikes to In Progress in @azure/cosmos Project Jun 22, 2022
@xirzec xirzec modified the milestones: 2022-07, 2022-08 Jul 8, 2022
@cjshelton
Copy link

Hi @xirzec - I see you've been somewhat active on this issue, so I'm just reaching out to see if you know any further details on this? I see it has been added to the "2022-08 milestone" - does this mean the issue has been acknowledged and planned to be fixed for then?

I'm seeing this exact issue when trying to save an object which has a Date type on it. Date does not seem to satisfy the JSONValue type.

Any help / comments on this would be appreciated to help us know whether we're going about this in the right way. Should we be manually converting the object to satisfy the JSONValue constraint by explicitly stringifying dates?

@agorischek
Copy link
Author

Should we be manually converting the object to satisfy the JSONValue constraint by explicitly stringifying dates?

Hey @cjshelton, I'm far from a Cosmos expert but my understanding is that in your case, yes, you'll want to convert your Date object to a string first. The JavaScript Date object has a bunch of methods on it (e.g. getMonth, getDay) and those can't be represented in JSON. In particular I'd encourage you to try .toISOString() on your date before storing.

@cjshelton
Copy link

Yep that's what I've been thinking. Thanks for taking the time to respond @agorischek

@xirzec
Copy link
Member

xirzec commented Jul 11, 2022

I'm also not a cosmos expert, but if it works with type casting it seems we might just need to fix our typings here?

I'll let @sajeetharan and @jay-most comment as to when this work will be able to be completed

@sajeetharan sajeetharan moved this from In Progress to In Review in @azure/cosmos Project Aug 3, 2022
@xirzec xirzec modified the milestones: 2022-08, 2022-09 Aug 9, 2022
@sajeetharan sajeetharan moved this from In Review to Spikes in @azure/cosmos Project Aug 23, 2022
@v1k1 v1k1 moved this from Spikes to In Progress in @azure/cosmos Project Oct 6, 2022
@v1k1 v1k1 moved this from In Progress to Spikes in @azure/cosmos Project Oct 6, 2022
@v1k1 v1k1 moved this from Spikes to In Progress in @azure/cosmos Project Oct 6, 2022
@v1k1 v1k1 moved this from In Progress to Delegated to Partners in @azure/cosmos Project Oct 6, 2022
@v1k1 v1k1 moved this from Delegated to Partners to Spikes in @azure/cosmos Project Oct 6, 2022
@sajeetharan sajeetharan moved this to In Progress in @azure/cosmos Project Oct 6, 2022
@xirzec xirzec modified the milestones: 2022-09, 2022-12 Nov 1, 2022
@xirzec xirzec added bug This issue requires a change to an existing behavior in the product in order to be resolved. Service Attention Workflow: This issue is responsible by Azure service team. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Mar 30, 2023
@ghost
Copy link

ghost commented Mar 30, 2023

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @bkolant-MSFT, @sajeetharan, @pjohari-ms.

Issue Details
  • Package Name: @azure/cosmos
  • Package Version: 3.16.1
  • Operating system: Windows ARM
  • nodejs
    • version: 16.15.1
  • browser
    • name/version:
  • typescript
    • version: 4.7.4
  • Is the bug related to documentation in

Describe the bug
The Upsert bulk operation accepts fewer input types than the upsert non-bulk method. The bulk approach requires a JSONObject, while the single upsert approach accepts unknown. This seems to prevent submitting documents that are instances of custom classes in bulk. Is this intentional?

To Reproduce

  1. Create a JavaScript object from a custom class
  2. Attempt to write it to Cosmos using Upsert operation via bulk method

Expected behavior
Object is written as document, just like as if upsert were called directly.

Additional context
Code sample:

  // Case 1: Plain object
  const document1 = { id: "1" };
  // TypeScript is fine with this and the operation succeeds
  await container.items.upsert(document1);

  // Case 2: Class
  class Document {
    constructor(public id: string) {}
  }
  const document2 = new Document("2");
  // TypeScript complains:
  //   Type 'Document' is not assignable to type 'JSONObject'.
  //     Index signature for type 'string' is missing in type 'Document'.ts(2322)
  await container.items.batch([
    {
      operationType: "Upsert",
      resourceBody: document2,
    },
  ]);

  // Case 3: Class with type casting
  const document3 = new Document("3");
  // Operation succeeds
  await container.items.bulk([
    {
      operationType: "Upsert",
      resourceBody: document3 as unknown as JSONObject,
    },
  ]);
Author: agorischek
Assignees: sajeetharan
Labels:

bug, customer-reported, Client, Cosmos, Service Attention

Milestone: azure-cosmos jan release

azure-sdk pushed a commit to azure-sdk/azure-sdk-for-js that referenced this issue Aug 1, 2023
Initial version of the code-signing dataplane service (Azure#22310)

* Initial version of the code-signing dataplane service

* Remove error objects since those come from the Azure Core namespace

* fix: added azure.codesigning.json resulting file

* fix: use newer version dependency syntax

* Rolling back version change per PR feedback

* fix: polling operation mapping per PR feedback

* Adding x-ms-examples for example validation

* fix: added custom-words per PR feedback

* Renamed parameter definition to match open api json definition

* fix: modified the api-version to synch up with the folder api-version

* Adding details json cadl validation

* refactor: moved from cadl to typespec

* chore: attach azure.codesiging.json definition

* chore: remove package-lock.json since its unneeded.

* feat: adding get sign eku operation & example

fix: sign long running operation

* chore: removed unused swagger.json generated

* chore: removing data-plane swagger generated, keeping just the typespec project

* Readding the swagger information for dataplane service

* fix: ci issues pointed

examples are not referenced on the swagger file
resources should not be read only

* added examples to typespec definition

* fix: examples

* more fixes for example.json operations

* even more fixes for example.json operations

* fix: minor

Remove optional parameter from region. since its required.
Added description to namespace definition
Removed optional parameter from operation status, since the id is required.
Added suppression of warning

* Added fixes to azure.codesigning.json generation

* Added required property of id to examples

* Switched property to a more descriptive naming for the eku response

* fix: moved examples to a non version specific folder

* fix: the compiler didnt liked not using the preview folder, rolling back

* feat: added oauth2 security definition

* fix: removed api-version from example path

* style: change casing of operations to camel case

fix: per guidance modified the getsignEku operation into listSignEku

* Update azure.codesigning.json generated by tsp compiler

* fix: modified ResourceAction to LongRunningResourceAction per feedback

* docs: added enum descriptions to each value on hash algorithms

* fix: pinned to current versions to avoid ci errors by using latest dependency

* Added prefix of StandardResourceOperations interface to Resource operations

* fix: sign operation should return 202 Accepted on example

* fix: sign operation should return 202 Accepted on example

* style: tsp format result

* refactor: removed unused mapping to parent resource

* refactor: removing typespec-python emitter

* Adding get sign root certificate operation

* fix: adding custom word rootcert

* fix: adding api version parameter

* Added resource provider folder per ci finding

* chore: removing package.json per PR feedback

* Renaming folder to remove "Microsoft." path per feedback from PR

see: https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/typespec-structure-guidelines.md for guidelines

* feat: expanded the typespec options to include the lang generation

* Fix: Based on review from API stewardship team

Added more detailed information for docs and summary of operations
Refactored getSignRootCertificate operation to use RpcOperation instead of custom operation
Pluralized the listSignEkus operation
Renamed models to provide more detailed description of their purposed
Eliminated the custom enum in favor of the typespec enum provided

* refactor: moved version to 2023-06-15 based on feedback of ACS Eng team

* refactor: renamed models following azure team feedback

* Update tspconfig.yaml

Applied latest schema of emitter options and parameters.

* fix: adding back missing custom words, fixing coffeelake casing

* docs: updated samples to include varied hashes to represent a more familiar request for customers

* docs: updated the docs clauses of the typespec file to avoid repetitions on the docs generated.

* fix: update README.md of data-plane generation to include the correct tag

* Regenerated azure.codesigning.json file

* Based on pr build feedback, updated custom words definition

* docs: per compiler feedback, adding doc definition for versioning

* fix: outdated examples of data-plane swagger

* fix: per PR feedback, using T payload for Foundations.OperationStatus

* chore: updated Azure.CodeSigning swagger generated using tsp v0.46.0

* fix: removed autorest from the dependencies per feedback

fix: added suppression for custom RPC operation

* Removing json examples for compliance with avocado pipeline

* fix: Adding examples to tsp directory

* Corrected samples

---------

Co-authored-by: Ray Chen <[email protected]>
@xirzec xirzec removed this from the 2023-10 milestone Nov 22, 2023
Copy link

Hi @agorischek, we deeply appreciate your input into this project. Regrettably, this issue has remained unresolved for over 2 years and inactive for 30 days, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 21, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jun 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. Service Attention Workflow: This issue is responsible by Azure service team.
Projects
None yet
Development

No branches or pull requests

5 participants