Skip to content
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

Adding missing equals Methods and Tests #2037

Merged
merged 9 commits into from
Apr 24, 2023
Merged

Conversation

jnt0r
Copy link
Contributor

@jnt0r jnt0r commented Dec 9, 2022

PR for #2034

@jnt0r
Copy link
Contributor Author

jnt0r commented Apr 12, 2023

Hi @tomakehurst, any feedback on this? Would be nice to have these features ;)

@tomakehurst
Copy link
Member

I'm curious as to why you feel these are required on these classes. Can you share some examples of where the lack of these is a problem?

@oleg-nenashev
Copy link
Member

@tomakehurst Without these methods, using hash-based, sets or sorted collections (or just sort()) may lead to undesired results. I can imagine that someone uses WireMock as an API and wants to organize data before passing the parameters (e.g. for UX purposes). But I am not sure whether it is the use-case @jnt0r has in mind

@jnt0r
Copy link
Contributor Author

jnt0r commented Apr 14, 2023

I'm building a layer above WireMock to dynamically create Mappings at runtime via proxyMappings. Every proxied request is recorded and then transformed to the format and checks we need and dynamically saved as a mapping. To detect if a mapping already exists, I'm using equals method. Without overriding these methods, you cannot compare mappings and parts of them.

@tomakehurst
Copy link
Member

@oleg-nenashev I understand in general what the impact of not including these methods is, but when I see people asking about them in internal classes where they're not needed for WireMock to work correctly I worry we're encouraging coupling to implementation details.

Having said that I think @jnt0r has a reasonable use case in mind here so I think we should merge this.

One comment about the change - I notice there are no test assertions for the hashCode() implementations - could we add these too?

@jnt0r
Copy link
Contributor Author

jnt0r commented Apr 15, 2023

Alright, thanks @tomakehurst.

I updated the PR and added the tests for the hashCode methods.

@tomakehurst tomakehurst merged commit 5c023b4 into wiremock:master Apr 24, 2023
7 checks passed
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.

None yet

3 participants