Skip to content

feat(http): allow caching requests with different origins between ser…#55274

Closed
jsaguet wants to merge 1 commit intoangular:mainfrom
jsaguet:feat/http-transfer-cache-origin-map
Closed

feat(http): allow caching requests with different origins between ser…#55274
jsaguet wants to merge 1 commit intoangular:mainfrom
jsaguet:feat/http-transfer-cache-origin-map

Conversation

@jsaguet
Copy link
Contributor

@jsaguet jsaguet commented Apr 9, 2024

…ver and client

Expose HTTP_TRANSFER_CACHE_ORIGIN_MAP in public api. This token can be used when different origins are used to access the same APIs between server and browser.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

  • Feature

What is the current behavior?

Angular SSR applications are sometimes deployed in an internal private network where APIs must be accessed with a different origin that the external urls used from the client application.

Currently, such requests cannot be reused in the client during hydration because the urls do not match in the transfer state.

Issue Number: #53702

What is the new behavior?

Such applications can provide an origin map between server and clients by providing HTTP_TRANSFER_CACHE_ORIGIN_MAP.

The requests made on the client to the external origin will be correctly reused.

// in app.server.config.ts
{
    provide: HTTP_TRANSFER_CACHE_ORIGIN_MAP,
    useValue: {
        'http://internal-domain:80': 'https://external-domain:443'
    }
}

// Alternative usage with dynamic values (depending on stage or prod environments)
{
    provide: HTTP_TRANSFER_CACHE_ORIGIN_MAP,
    useFactory: () => {
        const config = inject(ConfigService);
        return {
            [config.internalOrigin]: [config.externalOrigin],
        };
    }
}

The origin map must be provided only in the server application.
A Runtime error will be thrown when the token is present in the client-side application.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Apr 9, 2024
@jsaguet jsaguet force-pushed the feat/http-transfer-cache-origin-map branch from c98babc to 11a6410 Compare April 9, 2024 22:59
@jsaguet jsaguet force-pushed the feat/http-transfer-cache-origin-map branch from 11a6410 to 93bb3fd Compare April 9, 2024 23:14
@jsaguet
Copy link
Contributor Author

jsaguet commented Apr 9, 2024

There is something weird with the duplicate imports in the CI.
Nothing looks duplicated to me and linting is fine locally.
I'll check again tomorrow.

@jsaguet jsaguet force-pushed the feat/http-transfer-cache-origin-map branch 2 times, most recently from 69e2de6 to 20a94c5 Compare April 10, 2024 08:21
@jsaguet
Copy link
Contributor Author

jsaguet commented Apr 10, 2024

There is something weird with the duplicate imports in the CI. Nothing looks duplicated to me and linting is fine locally. I'll check again tomorrow.

Nevermind, #55012 was merged after I created my branch, which generated conflicts.

@jsaguet jsaguet force-pushed the feat/http-transfer-cache-origin-map branch 6 times, most recently from 048d66d to f1fe7c5 Compare April 10, 2024 11:58
@jsaguet jsaguet force-pushed the feat/http-transfer-cache-origin-map branch 3 times, most recently from d362742 to a4b798e Compare April 10, 2024 12:11
@jsaguet jsaguet force-pushed the feat/http-transfer-cache-origin-map branch from a4b798e to 48b9950 Compare April 10, 2024 12:28
@jsaguet jsaguet marked this pull request as ready for review April 10, 2024 12:49
@pullapprove pullapprove bot requested a review from atscott April 10, 2024 12:49
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsaguet thanks for addressing the feedback! I've left a few more comments.

@jsaguet jsaguet force-pushed the feat/http-transfer-cache-origin-map branch 5 times, most recently from 82d6213 to fb42226 Compare April 17, 2024 18:59
@jsaguet jsaguet force-pushed the feat/http-transfer-cache-origin-map branch from fb42226 to 06d06a3 Compare April 17, 2024 20:11
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, just left a couple proposals to improve error messages.

…ver and client

Expose `HTTP_TRANSFER_CACHE_ORIGIN_MAP` injection token in public api. This is useful when different origins are used to access the same APIs between server and browser.

Fixes angular#53702
@jsaguet jsaguet force-pushed the feat/http-transfer-cache-origin-map branch from 06d06a3 to 561225e Compare April 18, 2024 05:47
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsaguet thanks for addressing the feedback! The change looks great 👍

Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-for: public-api

@AndrewKushnir
Copy link
Contributor

@thePunderWoman could you please take a look at the public API changes when you get a chance?

Copy link
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

@thePunderWoman thePunderWoman added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 19, 2024
@alxhub
Copy link
Member

alxhub commented Apr 22, 2024

This PR was merged into the repository by commit 6f88d80.

@alxhub alxhub closed this in 6f88d80 Apr 22, 2024
Comment on lines +150 to +157
throw new RuntimeError(
RuntimeErrorCode.HTTP_ORIGIN_MAP_USED_IN_CLIENT,
ngDevMode &&
'Angular detected that the `HTTP_TRANSFER_CACHE_ORIGIN_MAP` token is configured and ' +
'present in the client side code. Please ensure that this token is only provided in the ' +
'server code of the application.',
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice you added this warning. 👍 Otherwise the (potentially private) URLs of APIs used only-on-the-server could be disclosed in the publicly available browser-JS-bundle.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: common/http Issues related to HTTP and HTTP Client detected: feature PR contains a feature commit server: http cache target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants