Fixes the bug #1680 that necessitated adding the 'name' parameter to the @model directive, regardless of the name of the GraphQL object type's name #1713
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## Why make this change? - Cherry-picks #1641 and #1650 to 0.8 release --------- Co-authored-by: Sean Leonard <[email protected]>
- Cherry picks PR #1644 to 0.8 release as a bug fix patch. --------- Co-authored-by: Ayush Agarwal <[email protected]>
…for Core module… (#1660) Cherry-Picks (#1656) to release/0.8 branch **Original Description:** ## Why make this change? - Closes #1655 - Health of [0.8.44-rc nupkg](https://nuget.info/packages/Microsoft.DataApiBuilder/0.8.44-rc) nupkg needs to be fixed ## What is this change? - To fix the NuGet generated in the `main` branch, `Product` and `Core` modules needs to be updated. `Product` module was introduced later and `0.8.*` NuGets do not have the Product module. So, the fix is broken into two PRs. - This PR adds changes only to the `Core` module. After merging to `main`, this PR can be ported over to `release/0.8` branch. - [PR #1657 ](#1657) adds the changes to `Product` module. - Sets `EmbedUntrackedSources` and `ContinuousIntegrationBuild` to `true` ## How was this tested? - [x] Manual Tests By applying the change on top of the `release/0.8` branch, the health of the NuGet generated was validated  --------- Co-authored-by: Shyam Sundar J <[email protected]>
- Cherry Picking #1663. ---------------- ## Why make this change? - With the overhaul of the config system using our own Write method while serializing objects, the config that is generated is misaligned with respect to indentation for all the keys under each entity name. For e.g.: In the following picture, the entity name `Todo` is indented with 4 leading spaces, subsequently its child key `source` should have been indented with 6 leading spaces, however its currently indented with only 2 spaces.  - This is a regression in 0.8.44-rc. ## What is this change? - Use the same writer to serialize the subkeys under the entity name so that the depth of indentation is maintained. ## How does this change fix the issue? - `WriteRawValue` writes the json string argument provided as is without doing any additional formatting on it. It doesn't utilize the `_currentDepth` private member of the writer to determine how to indent. - Prior to this change, we were generating the inner json string first then writing the generated inner json "as is". While generating that json on line 29 https://github.com/Azure/data-api-builder/blob/31b754251832ffebf4c4ce36ffcbead9baff6355/src/Config/Converters/RuntimeEntitiesConverter.cs#L29 Line 29 in [31b7542](31b7542) a new writer is being used by the `JsonSerializer.Serialize()` function, effectively resetting the `_currentDepth`. The `_currentDepth` of the writer in context was not getting honored. ## How was this tested? - Manual test, building solution and using `dab add` to simulate automatic writing of an entity to the config file. With the change, the indentation is fixed. 
Cherry picking #1669 Original description: ## Why make this change? - With the overhaul of config system PR #1402 , we started seeing redundant logs about which config file is being used. ## What is this change? - Store the environment based config file name found at construction time in the property `ConfigFileName`. - Remove Console writeline statements while checking for existence of file since this leads to multiple logs - one from CLI before starting the engine and second from within the engine itself when calling TryPargeConfig - Add a single log of the loaded file in CLI ## How was this tested? - Manually, using the `start` command of CLI as well as triggering the engine directly with argument: `--ConfigFileName` 1. Starting with default config: **Before**:  **After**:  2. Starting with user provided config: **Before**:  **After**:  3. Trying to start with missing file name - same behvior as before 
…fig file (#1678) ## Why make this change? - Closes #1675 - For Cosmos DB NoSQL database type, DAB CLI v0.8.49 generates a REST property within the Runtime section of the config file. However v0.7.6 does not generate this property. So, when the config file generated using v0.7.6 is used to start the engine with v0.8.49, the absence of the REST property causes the engine to throw exceptions. ## What is this change? - With `v0.8.49`, these values are assumed to have non-null values because the CLI explicitly writes out all the fields to the config files. So, `null` checks are not performed extensively in all the places. - However, when a config file that is generated using `v0.7.6` is used, it could be possible that some of the fields are `null`. The absence of `null` checks leads to `NullReferenceExceptions` in this case. - To avoid this, all the `Runtime` options - `Rest`, `GraphQL` and `Host` are initialized with default values when they are `null` after deserialization of the config file. ## How was this tested? - [x] Existing Unit Tests and Integration Tests - [x] Manual Tests # Sample Requests - Database Type: Cosmos DB NoSQL - Config file (Runtime section): ```json "runtime": { "graphql": { "enabled": true, "path": "/graphql", "allow-introspection": true }, "host": { "cors": { "origins": [ "http://localhost:5000" ], "allow-credentials": false }, "authentication": { "provider": "StaticWebApps" }, "mode": "development" } } ``` - Engine starts successfully:  - GraphQL Requests work successfully:   Note: This PR is directly targeted towards `release/0.8` branch (and the branch to make these changes was snapped off of `release/0.8`) because the current `main` has additional changes that are related to `0.9` and porting it over to `release/0.8` branch might not be possible. --------- Co-authored-by: Aniruddh Munde <[email protected]>
## Why make this change? - As per the JSON config schema, not all properties in the config file are mandatory. - With #1402, in versions 0.8.49+ those properties that are not explicitly set, are initialized with defaults before writing to the file system. - However, config files generated using version 0.7.6 or lesser, may not have these properties initialized to their defaults when writing to the file system. Using config files generated with <= 0.7.6, to start engine with 0.8.49 would therefore lead to null reference exceptions when the optional properties are dereferenced. - This PR is to ensure such null reference exceptions are avoided when starting engine with configs that don't have optional properties ## What is this change? - Modify object model constructors to accept nullable arguments for optional properties - represented as fields in the record types. If passed in argument is null, initialize the field with default values. Do this recursively for sub-properties. - To force JSON deserializer to pick up the custom constructor, add `IncludeFields = true` to `JsonSerializerOptions` - Default value of host mode is changed to `development` since `allow-introspection` has a default value of `true` ## How was this tested? - Removed optional properties from config and ensured engine could be started without them. - [X] Unit Tests ## Sample Request(s) - remove `runtime` section from config, starting engine with 0.8.49 leads to NullReferenceException. - After the fix, starting engine is successful even when `runtime` section is absent in the config.
- Cherry Picks #1691 into release/0.8 --------- Co-authored-by: Abhishek Kumar <[email protected]>
- Cherry Picks #1681 --------- Co-authored-by: abhishekkumams <[email protected]>
…where the 'name' attribute is not explicitly added in the schema
neeraj-sharma2592
requested review from
mbhaskar,
Aniruddh25,
JelteF,
seantleonard,
Mathos1432,
gledis69,
aaronburtle,
milismsft,
tarazou9,
ayush3797,
abhishekkumams,
aaronpowell and
severussundar
as code owners
|
Will be creating another pull request taking main as base |
neeraj-sharma2592
deleted the
dev/neeraj-sharma2592/fixNameDirectiveInvalidArgException
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why make this change?
What is this change?
How was this tested?