Skip to content

Ports the patch fix for missing global REST section in config file (for 0.8.* version) to the main branch#1712

Merged
ayush3797 merged 1 commit intomainfrom
dev/shyamsundarj/port-0.8-patches-to-main
Sep 14, 2023
Merged

Ports the patch fix for missing global REST section in config file (for 0.8.* version) to the main branch#1712
ayush3797 merged 1 commit intomainfrom
dev/shyamsundarj/port-0.8-patches-to-main

Conversation

@severussundar
Copy link
Copy Markdown
Contributor

Ports the patch fix PR #1678 which was directly merged into release/0.8 to the main branch so that it is avaiable for future versions

Original PR description:

Why make this change?

  • Closes [Bug]: Object reference not set to Instance of an object #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?

  • Existing Unit Tests and Integration Tests
  • Manual Tests

Sample Requests

  • Database Type: Cosmos DB NoSQL
  • Config file (Runtime section):
 "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:
    image

  • GraphQL Requests work successfully:
    image

image

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.

…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:

![image](https://github.com/Azure/data-api-builder/assets/11196553/dc8769d4-a01a-485d-8800-ef5a311f4554)

- GraphQL Requests work successfully:

![image](https://github.com/Azure/data-api-builder/assets/11196553/111de37d-172a-4bb8-b3c9-686ee4061608)


![image](https://github.com/Azure/data-api-builder/assets/11196553/b34bd5df-0c01-45b6-a708-7c1bcb5243b5)




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]>
@ayush3797 ayush3797 merged commit a5f9db0 into main Sep 14, 2023
@ayush3797 ayush3797 deleted the dev/shyamsundarj/port-0.8-patches-to-main branch September 14, 2023 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working config changes related to config

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Object reference not set to Instance of an object

3 participants