fix(java): java engine now passes UTF-8 encoded strings correctly on Windows #202
Conversation
| @@ -69,14 +70,10 @@ public UnleashEngine(List<IStrategy> customStrategies, IStrategy fallbackStrateg | |||
| } | |||
| } | |||
|
|
|||
| public void takeState(String toggles) throws YggdrasilInvalidInputException { | |||
| YggResponse<Void> response = read(yggdrasil.takeState(this.enginePtr, toggles), | |||
| public void takeState(String toggles) throws YggdrasilInvalidInputException, YggdrasilError { | |||
There was a problem hiding this comment.
All the changes from here to line 148 are basically the same. Use read now directly returns the object requested rather than the Yggdrasil wrapping response. If the method passes a string to Yggdrasil, that's now encoded explicitly as UTF-8 before send
| * Handle reading from a pointer into an YggdrasilResponse and unwrapping that | ||
| * to an object | ||
| */ | ||
| private <T> T read(Pointer pointer, TypeReference<YggResponse<T>> clazz) throws YggdrasilError { |
There was a problem hiding this comment.
Just some convenience, this method now always bubbles the actual error response from Yggdrasil and also unwraps the response before returning it to the caller so that the caller doesn't have to check for errors, it can just expect an exception
| /** Handle reading from a pointer into a String and mapping it to an object */ | ||
| private <T> T read(Pointer pointer, TypeReference<T> clazz) { | ||
| private <T> T readRaw(Pointer pointer, TypeReference<T> clazz) { |
There was a problem hiding this comment.
Maintained for cases where Yggdrasil returns static entities that are not encoded with the normal response formatting (JSON blob signalling if the call succeeded etc)
| @@ -5,6 +5,7 @@ | |||
| import com.fasterxml.jackson.databind.DeserializationFeature; | |||
| import com.fasterxml.jackson.databind.ObjectMapper; | |||
| import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; | |||
| import com.sun.jna.Memory; | |||
There was a problem hiding this comment.
I just spotted this and I believe com.sun.jna is only part of Oracle's distribution of Java. It can be added as a dependency alongside Unleash but I'm not sure if we want to go this route.
There was a problem hiding this comment.
It comes from this library: https://github.com/java-native-access/jna, which is LGPL licensed and we've been testing this on OpenJDK, I think it's fine. This is already an import, it's just new in this file
We may yeet JNA for different reasons though
* chore(java): add some tests for camel case mapping
sighphyre
deleted the
fix/java-engine-now-passes-utf-strings-on-windows
branch
This forces the Java engine to encode its strings as UTF-8 pointers rather than whatever the JVM feels like (thank you, Windows, very nice). To figure out what was going on here, there's some redesign to the way the pointers are materialized so that they bubble the Rust errors to the caller