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

Implemented the getting environment from remote URL #4967

Merged
merged 7 commits into from
Aug 7, 2020
Merged

Conversation

mehmet-erim
Copy link
Contributor

@mehmet-erim mehmet-erim commented Aug 5, 2020

Resolves #4386

Remote environment configuration can be added to the environment like below:

import { Config } from '@abp/ng.core';

export const environment = {
  production: false,
  hmr: false,
  remoteEnv: {
    url: '/assets/appsettings.json',
    method: 'GET',
    headers: {}
  }
} as Config.Environment;

@mehmet-erim mehmet-erim added this to the 3.1 milestone Aug 5, 2020
@mehmet-erim mehmet-erim requested a review from armanozak August 5, 2020 15:00
@mehmet-erim mehmet-erim marked this pull request as ready for review August 6, 2020 07:23
@mehmet-erim mehmet-erim merged commit 5c16926 into dev Aug 7, 2020
@mehmet-erim mehmet-erim deleted the feat/4386 branch August 7, 2020 15:18
@olicooper
Copy link
Contributor

olicooper commented Aug 7, 2020

My implementation (see here) allowed for partial replacement and appending of nested properties using a package called deepmerge. Is this something you'd be able to add in to make it more flexible and similar to .NET Core's appsettings.json replacement?

I could have the default environment.ts like this:

{
  recaptcha: { enabled: false, siteKey: 'AAAA' }
}

And my /assets/appsettings.json like this:

{
  recaptcha: { enabled: true  },
  anotherSetting: "hi"
}

Which results in:

{
  recaptcha: { enabled: true, siteKey: 'AAAA' },
  anotherSetting: "hi"
}

You don't have to use the deepmerge package, but if you do then I'm not sure how you'd want to handle array merging...
https://github.com/TehShrike/deepmerge#arraymerge

@mehmet-erim
Copy link
Contributor Author

Hi @olicooper

Thanks for your suggestion. We'll consider this.

@olicooper
Copy link
Contributor

Could you also consider the option to suppress the RestError and gracefully fall back to the default environment configuration if the remoteEnv is not available? Maybe a simple console debug/warning log rather than passing it to the error handler and showing to the end user?

@olicooper
Copy link
Contributor

olicooper commented Aug 13, 2020

I have just tried this on a staging environment and found an issue on the Angular UI...

I'm not sure why but the request to /.well-known/openid-configuration is made before the remoteEnv is configured. so the request is made to the wrong API (https://api.app.example.com instead of https://api.staging.example.com).

environment.prod.ts:

{
  oAuthConfig: {
    issuer: 'https://api.app.example.com',
    // etc...
  },
  apis: {
    default: {
      url: 'https://api.app.example.com',
    },
  }
}

appsettings.json:

{
  "oAuthConfig": {
    "issuer": "https://api.staging.example.com",
    // etc...
  },
  "apis": {
    "default": {
      "url": "https://api.staging.example.com"
    }
  }
}

I am using @abp/ng.core 3.1.0-preview20200813

@mehmet-erim
Copy link
Contributor Author

Could you also consider the option to suppress the RestError and gracefully fall back to the default environment configuration if the remoteEnv is not available? Maybe a simple console debug/warning log rather than passing it to the error handler and showing to the end user?

We will create an injection token to be able to get catch error function from the AppModule. See the related issue: #4992

I will check it. Thanks for the information.

@olicooper
Copy link
Contributor

Perfect, great work! Thank you 👍

@olicooper
Copy link
Contributor

@mehmet-erim I think I have fixed the problem.. #5058

yinchang0626 pushed a commit to yinchang0626/abp.ng.packages that referenced this pull request Sep 4, 2020
Relates to abpframework/abp#4967
When using `remoteEnv`, OAuth was being configured with the default `environment.ts` settings rather than the one loaded from the `remoteEnv`. This should fix the problem.
bug originally mentioned here: abpframework/abp#4967 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Angular: Read settings from Json files
3 participants