8000 ParamConverterFactory should use LinkedHashSet to preserve ordering by chkal · Pull Request #3955 · eclipse-ee4j/jersey · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

ParamConverterFactory should use LinkedHashSet to preserve ordering #3955

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

Merged
merged 1 commit into from
Oct 4, 2018

Conversation

chkal
Copy link
Contributor
@chkal chkal commented Oct 3, 2018

This pull request has been migrated from jersey/jersey#3669. It fixes the first issue described in #3670.

In a nutshell: If you provide a custom ParamConverterProvider for a data type for which there is also a builtin one provided by Jersey, a random one wins and gets executed. This changes even between deployments. This issue is caused by a HashSet in ParamConverterFactory which should actually be a LinkedHashSet to preserve the ordering.

…clipse-ee4j#3670)

Signed-off-by: Christian Kaltepoth <christian@kaltepoth.de>
@arjantijms
Copy link
Contributor

Maybe it's unrelated, but this test is failing now:

Failed tests: 
  BroadcasterTest.test:193 Waiting for resultsB1 to be complete failed.

@senivam
Copy link
Contributor
senivam commented Oct 4, 2018

shall be unrelated.. however the full PR duplicates #3925 or am I missing something?

But explanation here is really brilliant because I was investigating this yesterday and I was not sure if it helps (LinkedHashSet I mean)

@chkal
Copy link
Contributor Author
chkal commented Oct 4, 2018

@senivam Yes, looks like #3925 is actually fixing the same issue.

@senivam
Copy link
Contributor
senivam commented Oct 4, 2018

but this gained 2 approves

@jansupol jansupol merged commit 02631b4 into eclipse-ee4j:master Oct 4, 2018
@chkal chkal deleted the provider-set branch October 4, 2018 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0