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 hasgetCookieStore() method.
RequestBuilder hassetCookieStore() method to set cookie store -> cookie store at request level isnull 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 added2 commits

July 31, 2018 11:59

Enable cookie store at request level

105f388

Add cookie to request cookie store when it's present

ed0895b

tranchitam mentioned this pull request

Jul 31, 2018

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

Open

slandelle requested changes

Aug 20, 2018

View reviewed changes

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

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

		// This MUST BE called before Redirect30xInterceptor because latter assumes cookie store is already updated
		CookieStorecookieStore =config.getCookieStore();

Copy link

Contributor

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

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

		CookieStorecookieStore =config.getCookieStore();
		if (cookieStore !=null) {
		CookieStorerequestCookieStore =request.getCookieStore();
		if (cookieStore !=null \|\|requestCookieStore !=null) {

Copy link

Contributor

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

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

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

Choose a reason for hiding this comment

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

Restored

Copy link

Contributor

slandelle commentedAug 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 force-pushed themaster branch 5 times, most recently from6ea11f4 tof8fab66Compare

February 7, 2020 12:25

tranchitam added2 commits

April 30, 2020 17:01

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

f2a96bd

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

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

dc2e23c

…l cookie store

Copy link

Author

tranchitam commentedApr 30, 2020

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

Copy link

mkurz commentedMay 21, 2020

@slandelle Can you have a look?

Copy link

mkurz commentedMay 21, 2020

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

Copy link

leomaiwego commentedFeb 14, 2022

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

Copy link

Author

tranchitam commentedMar 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

Movatterモバイル変換

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 commentedJul 31, 2018• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

slandelleAug 20, 2018

Choose a reason for hiding this comment

Uh oh!

tranchitamApr 30, 2020

Choose a reason for hiding this comment

Uh oh!

slandelleAug 20, 2018

Choose a reason for hiding this comment

Uh oh!

tranchitamApr 30, 2020

Choose a reason for hiding this comment

Uh oh!

slandelleAug 20, 2018

Choose a reason for hiding this comment

Uh oh!

tranchitamApr 30, 2020

Choose a reason for hiding this comment

Uh oh!

slandelle commentedAug 27, 2018• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

tranchitam commentedApr 30, 2020

Uh oh!

mkurz commentedMay 21, 2020

Uh oh!

mkurz commentedMay 21, 2020

Uh oh!

leomaiwego commentedFeb 14, 2022

Uh oh!

tranchitam commentedMar 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tranchitam commentedJul 31, 2018•
edited
Loading

slandelle commentedAug 27, 2018•
edited
Loading