Skip to content

normalization should not change KeyDataPair to NameValuePair #533

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.

Sign up for GitHub

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
Merged
rbri merged 2 commits into from
Dec 21, 2022
Merged

normalization should not change KeyDataPair to NameValuePair #533

rbri merged 2 commits into from
Dec 21, 2022

Conversation

@thuri
Copy link
Contributor

@thuri thuri commented Dec 20, 2022 •
edited
Loading

@thuri thuri changed the title normalization should not change KeyDataPair to NameValuePair Fixes #532 normalization should not change KeyDataPair to NameValuePair Dec 20, 2022
rbri
rbri requested changes Dec 21, 2022
View reviewed changes
Copy link
Member

@rbri rbri left a comment

Choose a reason for hiding this comment

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

Looks reasonable for me, thanks for this.
Please add yourself as author to all changed files and maybe also to the pom (if you are not already in the list)

@rbri
Copy link
Member

rbri commented Dec 21, 2022

Jenkins will have a look at this....

@rbri
Copy link
Member

rbri commented Dec 21, 2022

sorry we are still at java8 :-)

/var/lib/jenkins/workspace/htmlunit_PR-533/src/test/java/com/gargoylesoftware/htmlunit/WebRequestTest.java:[437,42] cannot find symbol
symbol:   method of(com.gargoylesoftware.htmlunit.util.NameValuePair,com.gargoylesoftware.htmlunit.util.KeyDataPair)
location: interface java.util.List

@thuri
Copy link
Contributor Author

thuri commented Dec 21, 2022

Yeah I've seen that but somehow eclipse didn't warn me when i used Java 11 features.
I corrected the problematic part. At least I hope so

rbri
rbri reviewed Dec 21, 2022
View reviewed changes
src/main/java/com/gargoylesoftware/htmlunit/util/KeyDataPair.java Outdated
public KeyDataPair normalized() {
final KeyDataPair pair = new KeyDataPair(
this.getName(),
this.fileObject_ != null ? this.fileObject_ : new File(this.getValue()),
Copy link
Member

@rbri rbri Dec 21, 2022

Choose a reason for hiding this comment

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

new File(...) might do some normalization on the file name leading in the end to some new value -> Testcase?
maybe it is better to have a private constructor that allows to pass the values 1:1?

What do you think?

@rbri
Copy link
Member

rbri commented Dec 21, 2022

tests are looking good !

@thuri
private copy constructor for KeyDataPair
98368c2 
this should prevent manipulation of fields and make sure that
"normalized" instances are identical copies if the source object is
normalized already
@thuri
Copy link
Contributor Author

thuri commented Dec 21, 2022

@rbri please have a look at the additional commit. Perhaps that is what you had in mind ...

@rbri rbri merged commit ecfbc91 into HtmlUnit:master Dec 21, 2022
@rbri
Copy link
Member

rbri commented Dec 21, 2022

i have nothing in mind ;-)
thanks a lot - will inform when the new snapshot is availble

and all the best for 2023

@thuri
Copy link
Contributor Author

thuri commented Dec 21, 2022

Thanks a lot and the all the best for you too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rbri rbri rbri requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

RequestParameter normalization replaces KeyDataPair with NameValuePair

2 participants

@thuri @rbri