Feature/enable cookie store at request level #1567

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

Jump to bottom

Open

tranchitam wants to merge 4 commits into AsyncHttpClient:main

from tranchitam:feature/enable-cookie-store-at-request-level

Open

Feature/enable cookie store at request level #1567

tranchitam wants to merge 4 commits into AsyncHttpClient:main from tranchitam:feature/enable-cookie-store-at-request-level

Conversation

@tranchitam

Copy link

@tranchitam tranchitam commented Jul 31, 2018 •

edited

Loading

Motivations:

Enable the ability to pass CookieStore at request level -> cookies between different requests having the same URI will be independent and will not override each other.
Still keep global CookieStore from config.

Changes:

Request has getCookieStore() method.
RequestBuilder has setCookieStore() method to set cookie store -> cookie store at request level is null by default.
When http responses come -> add cookies to request cookie store if it's present, otherwise add cookies to global cookie store.
When setting cookies for redirect request -> if request cookie store is present, get cookies from request cookie store and set them to redirect request. Otherwise, if global cookie store is present, get cookies from global cookie store and and them to redirect request.

Results:

Still keep global cookie store.
If request has its own cookie store -> cookies for different requests having the same URI will be independent and not override each other

Related issue:

#1565

tranchitam added 2 commits

July 31, 2018 11:59

@tranchitam


 Enable cookie store at request level

105f388

@tranchitam


 Add cookie to request cookie store when it's present

ed0895b

@tranchitam tranchitam mentioned this pull request

Jul 31, 2018

Provide a way to have request level control over CookieStore (currently only global) #1565

Open

slandelle

slandelle requested changes

Aug 20, 2018

View reviewed changes

client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java Outdated

Realm realm = request.getRealm() != null ? request.getRealm() : config.getRealm();

// This MUST BE called before Redirect30xInterceptor because latter assumes cookie store is already updated

CookieStore cookieStore = config.getCookieStore();

Copy link

Contributor

@slandelle slandelle Aug 20, 2018

Choose a reason for hiding this comment

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

CookieStore cookieStore = request.getCookieStore() != null ? request.getCookieStore() : config.getCookieStore() ;

Copy link

Author

@tranchitam tranchitam Apr 30, 2020

Choose a reason for hiding this comment

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

I already updated.

client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java Outdated

CookieStore cookieStore = config.getCookieStore();

if (cookieStore != null) {

CookieStore requestCookieStore = request.getCookieStore();

if (cookieStore != null || requestCookieStore != null) {

Copy link

Contributor

@slandelle slandelle Aug 20, 2018

Choose a reason for hiding this comment

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

Please restore

Copy link

Author

@tranchitam tranchitam Apr 30, 2020

Choose a reason for hiding this comment

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

Restored

client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java Outdated

if (requestCookieStore != null) {

requestCookieStore.add(request.getUri(), c);

}

Copy link

Contributor

@slandelle slandelle Aug 20, 2018

Choose a reason for hiding this comment

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

Please restore

Copy link

Author

@tranchitam tranchitam Apr 30, 2020

Choose a reason for hiding this comment

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

Restored

@slandelle

Copy link

Contributor

slandelle commented Aug 27, 2018 •

edited

Loading

@tranchitam Did you get a chance to have a look at my comments. The idea is to be able to override the global CookieStore with one scoped at request level. Those 2 are exclusive and only one should be updated.

@slandelle slandelle force-pushed the master branch 5 times, most recently from 6ea11f4 to f8fab66 Compare

February 7, 2020 12:25

tranchitam added 2 commits

April 30, 2020 17:01

@tranchitam


 Merge branch 'master' of github.com:AsyncHttpClient/async-http-client...

f2a96bd

... into feature/enable-cookie-store-at-request-level

@tranchitam


 Update request cookie store when it's present, otherwise update globa...

dc2e23c

...l cookie store

@tranchitam

Copy link

Author

tranchitam commented Apr 30, 2020

Hi @slandelle, I already updated the pull request to fix your comments. Please take a look. Thanks

@mkurz

Copy link

mkurz commented May 21, 2020

@slandelle Can you have a look?

@mkurz

Copy link

mkurz commented May 21, 2020

Also there is #1610 now, however seems liks it's just re-using the commits from this pull request here.

@leomaiwego

Copy link

leomaiwego commented Feb 14, 2022

@slandelle Could you please take a look to this one?

@tranchitam

Copy link

Author

tranchitam commented Mar 3, 2022

Hi @slandelle, @TomGranot have you got time to take a look into this pull request again? If there is no issue, we really want to merge this PR to master as soon as possible.

Labels

None yet

4 participants

@tranchitam @slandelle @mkurz @leomaiwego

Feature/enable cookie store at request level #1567

Are you sure you want to change the base?

Feature/enable cookie store at request level #1567

Uh oh!

Conversation

@tranchitam tranchitam commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

@slandelle slandelle Aug 20, 2018

Choose a reason for hiding this comment

Uh oh!

@tranchitam tranchitam Apr 30, 2020

Choose a reason for hiding this comment

Uh oh!

@slandelle slandelle Aug 20, 2018

Choose a reason for hiding this comment

Uh oh!

@tranchitam tranchitam Apr 30, 2020

Choose a reason for hiding this comment

Uh oh!

@slandelle slandelle Aug 20, 2018

Choose a reason for hiding this comment

Uh oh!

@tranchitam tranchitam Apr 30, 2020

Choose a reason for hiding this comment

Uh oh!

slandelle commented Aug 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tranchitam commented Apr 30, 2020

Uh oh!

mkurz commented May 21, 2020

Uh oh!

mkurz commented May 21, 2020

Uh oh!

leomaiwego commented Feb 14, 2022

Uh oh!

tranchitam commented Mar 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

@tranchitam tranchitam commented Jul 31, 2018 •

edited

Loading

slandelle commented Aug 27, 2018 •

edited

Loading