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

Update behaviour of ?case=original query to /service/javascript-app - return raw contents #6813

Merged

Conversation

eddyashton
Copy link
Member

Resolves #6733.

A sensible use case of /gov/javascript-app is to check that the stored app matches the bundle you submitted. As noted in #6433, we can't do a direct equality check on this, because we've recased the response fields to match the Azure API standards. We added a ?case=original field, which falls back to the macro-generated to_json conversion.

However, as #6733 notes this is still imprecise - the generated C++ conversion function omits some fields if they match default values, so doesn't necessarily return every field from the original bundle. We could try explicitly writing every field (in the classic/original case), but would then be potentially returning more fields than were specified in the original bundle, which would still prohibit a direct equality check.

Instead, this PR uses a raw handle to access the metadata table, and returns that value without any magic parsing conversions. So it matches exactly the bundle that was written by the constitution, with minimal further attempts in C++ to re-serialise/parse/validate this.

There are a few caveats even with this approach:

  • We still convert this to an nlohmann::json object in C++, to simplify serialisation into the HTTP reponse body. This means we're checking that the C++ JSON parser thinks it is valid JSON, and re-ordering fields within objects.
  • There is some normalisation done by the constitution before the bundle is written to the KV, that we can't undo in this accessor. Specifically all HTTP methods were upper-cased (where they're lower-cased in app.json), and module paths are prefixed with / (for historical, potentially unjustified, reasons).

These caveats mean that even with all this we still can't do a direct equality check - you can't assert that the response to GET /gov/service/javascript-app?case=original matches the contents of app.json. But I think this is as close as we can get.

@achamayou achamayou added this pull request to the merge queue Feb 6, 2025
Merged via the queue into microsoft:main with commit ef1ca8e Feb 6, 2025
18 checks passed
@achamayou achamayou deleted the case_original_returns_raw_contents branch February 6, 2025 21:43
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 this pull request may close these issues.

mode=readwrite key/value pair gets skipped in the output of /gov/service/javascript-app?case=original
2 participants