Skip to content

sanitizeValues() tries to mutate the input settings data #1385

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

Closed
FyiurAmron opened this issue Nov 9, 2023 · 2 comments
Closed

sanitizeValues() tries to mutate the input settings data #1385

FyiurAmron opened this issue Nov 9, 2023 · 2 comments
Assignees
Labels
bug Something isn't working good-first-issue Good for newcomers
Milestone

Comments

@FyiurAmron
Copy link

Issue Description

sanitizeValues() tries to mutate the input settings data, which is visible mostly for customAnnotationsMapping and directiveAnnotationsMapping; if the input data is immutable (e.g. supplied via mapOf() etc.), the result is a not-too-helpful UnsupportedOperationException.

The solution would be for the sanitization method to produce a mutated copy of the whole map instead, and re-assign that copy via mappingConfig.setDirectiveAnnotationsMapping() etc. instead of trying to mutate in place.

A worse solution would be to explicitly require a mutable input parameter (e.g. mutable map) in the affected cases, although this would be a serious code smell IMVHO.

Steps to Reproduce

        directiveAnnotationsMapping = mapOf(
            "foo" to listOf(
                "@bar",
            ),
        )

with Kotlin DSL e.g.

Expected Result

Should work.

Actual Result

UnsupportedOperationException due to attempted entry.setValue() on the map.

Your Environment and Setup

  • graphql-java-codegen version: 5.8.0
  • Build tool: Gradle 8.4
  • Mapping Config: see above
@FyiurAmron FyiurAmron added the bug Something isn't working label Nov 9, 2023
@kobylynskyi kobylynskyi added this to the 5.9.0 milestone Nov 9, 2023
@kobylynskyi
Copy link
Owner

@FyiurAmron, since you have already investigated this, would you be able to contribute? Otherwise, I might be able to pick this up some time next week.
Thanks!

@FyiurAmron
Copy link
Author

@kobylynskyi honestly, hard to say. I'd gladly do a PR, since it looks simple enough, but the earliest time I'd be able to handle this would be around the end of the next week as well. If this was a blocker, I'd probably do it even tomorrow, but thankfully it isn't :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good-first-issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants