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

build: update to Micronaut 4 #331

Merged
merged 14 commits into from
Dec 20, 2022
Merged

build: update to Micronaut 4 #331

merged 14 commits into from
Dec 20, 2022

Conversation

sdelamo
Copy link
Contributor

@sdelamo sdelamo commented Nov 28, 2022

No description provided.

…318)

* use buildSrc scripts to avoid unnecessary duplication
… 2.3-groovy-4.0, github workflows to Java 17 (#323)
@sdelamo
Copy link
Contributor Author

sdelamo commented Nov 28, 2022

@timyates could you look into this?

@timyates
Copy link
Contributor

timyates commented Dec 1, 2022

Still failing, I suspect it's an issue in core

Previously conversion from a byte[] would return Optional.empty

This would trigger the deserialization from the value in the attribute map

Now conversion would return a String (which would be the JSON representation of the object)

So we end up with strings in the session wrapped in double quotes.

This change gets the value as an Object type, and if this is a byte array, deserializes it from JSON
@timyates
Copy link
Contributor

timyates commented Dec 15, 2022

@sdelamo Fixed.

I had to pass the ConversionService in to the RedisSessionStore, as previously conversion from a byte[] would return Optional.empty. This would trigger the deserialization from the value in the attribute map. Now in Micronaut 4.0.0, conversion returns a String (which would be the JSON representation of the object), so we ended up with strings in the session wrapped in double quotes. (@graemerocher does this look ok?)

This change gets the value as an Object type, and if this is a byte array, deserializes it from JSON as we used to rely on the value being null conversion context not being able to , which meant I had to change the public constructor

After a discussion in the meeting, there was no preference to simply making the change over deprecating the old constructor, so I went with the former and added it to the accepted-api-changes

@timyates timyates requested a review from wetted December 15, 2022 16:01
gradle.properties Outdated Show resolved Hide resolved
gradle/libs.versions.toml Outdated Show resolved Hide resolved
redis-lettuce/build.gradle Outdated Show resolved Hide resolved
@sdelamo sdelamo merged commit 6d36a7a into master Dec 20, 2022
@sdelamo sdelamo deleted the 6.0.x branch December 20, 2022 09:51
@sonarcloud
Copy link

sonarcloud bot commented Dec 20, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants