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

[FeatureRequest] Allow customizing the Gson instance for Json serialization #424

Open
CrawX opened this issue Mar 19, 2021 · 18 comments · May be fixed by #704
Open

[FeatureRequest] Allow customizing the Gson instance for Json serialization #424

CrawX opened this issue Mar 19, 2021 · 18 comments · May be fixed by #704

Comments

@CrawX
Copy link

CrawX commented Mar 19, 2021

Hi all,

Current problem
The Gson instance used by RpcUnspecifiedDataTarget is hardcoded in RpcJsonDataSource



This means that customizing the way the runtime Json-serializes objects (especially dates) is impossible. See #419 and this on stackoverflow.

Workaround
I came up with a dirty workaround to replace the used Gson instance with the spring-provided one via Reflection:

AzureGsonConfig.java:

@Configuration
@ConditionalOnClass(RpcJsonDataSource.class)
public class AzureGsonConfig {

  @SneakyThrows
  public AzureGsonConfig(Gson gson) {
    final Field gsonField = RpcJsonDataSource.class.getField("gson");
    gsonField.setAccessible(true);
    final Field modifiersField = Field.class.getDeclaredField("modifiers");
    modifiersField.setAccessible(true);
    modifiersField.setInt(gsonField, gsonField.getModifiers() & ~Modifier.FINAL);

    gsonField.set(null, gson);
  }
}

This requires a compile-time dependency like this

compileOnly files('libs/azure-functions-java-worker.jar')

(referencing the runtime as a jar file because I couldn't find it in maven central).

Having replaced the Gson instance means you can have a second configuration that customizes the date serialization behaviour of the one from spring.

GsonConfig.java:

@Configuration
public class GsonConfig {

  @Bean
  GsonBuilderCustomizer dateFormatGsonBuilderCustomizer() {
    return gsonBuilder -> {
      gsonBuilder.registerTypeAdapter(
          LocalDate.class, new LocalDateAdapter()
      );
      gsonBuilder.registerTypeAdapter(
          LocalDateTime.class, new LocalDateTimeAdapter()
      );
    };
  }

  static class LocalDateAdapter extends TypeAdapter<LocalDate> {

    @Override
    public void write(final JsonWriter jsonWriter, final LocalDate localDate) throws IOException {
      if (localDate == null) {
        jsonWriter.nullValue();
      } else {
        jsonWriter.value(localDate.toString());
      }
    }

    @Override
    public LocalDate read(final JsonReader jsonReader) throws IOException {
      if (jsonReader.peek() == JsonToken.NULL) {
        jsonReader.nextNull();
        return null;
      } else {
        return LocalDate.parse(jsonReader.nextString());
      }
    }
  }

  static class LocalDateTimeAdapter extends TypeAdapter<LocalDateTime> {

    @Override
    public void write(final JsonWriter jsonWriter, final LocalDateTime localDateTime) throws IOException {
      if (localDateTime == null) {
        jsonWriter.nullValue();
      } else {
        jsonWriter.value(localDateTime.toString());
      }
    }

    @Override
    public LocalDateTime read(final JsonReader jsonReader) throws IOException {
      if (jsonReader.peek() == JsonToken.NULL) {
        jsonReader.nextNull();
        return null;
      } else {
        return LocalDateTime.parse(jsonReader.nextString());
      }
    }
  }
}

While this works, it's not very clean and can break easily.

Alternative
Serialize the objects to strings in the functions or handlers and return them as such. Personally, I don't like this approach.

Feature request
Somehow allow injecting or customizing the Gson instance used in the azure-functions-worker runtime.

@vetler
Copy link

vetler commented Jun 17, 2022

+1

@kaibocai
Copy link
Member

Hi @CrawX , @vetler , thanks for opening this issue. This is a hard issue we are actively working on. We are trying to remove gson dependency from java worker. We will try to roll out the feature as soon as possible. Will keep you updated. Thank you.

@CrawX
Copy link
Author

CrawX commented Jan 3, 2023

Unfortunately, using the workaround that I posted is getting significantly more complicated with JDK12 and higher.
With JDK17 being GA with Azure functions, this is the only thing keeping me from switching to JDK17.

Is there an update on when this could be fixed?

@kaibocai
Copy link
Member

Hi @CrawX, thanks for raising the issue for request this feature. I am trying to see how much benefit this feature will bring to our user.
So you can always bind the payload to a plain String type and customize your gson instance in the function app to deserializeyour playload right. This won't have much difference compared with we providing a hook for user to customize their gson instance. Correct me if I understand it wrongly. Thanks.

@CrawX
Copy link
Author

CrawX commented Feb 17, 2023

I don't consider using a String to be a viable alternative. If you use this with Spring Boot, it feels very awkward and not how you would expect Spring to behave at all.

Using a String directly would require all individual users either implement the necessary boilerplate code for serialization themselves, either as a generic abstraction or in each function. The first option causes a lot of room for error and duplication amongst users, the second one feels very awkward.

The fact that this is not configurable right now seems to be more a implementation detail / limitation than a conscious design decision.

@CrawX
Copy link
Author

CrawX commented Jun 9, 2023

Can you provide an update on this @kaibocai? It looks like Spring Data cosmos will be compatible with JDK17 soon, it'd be great if this could get integrated properly so I can ditch my workaround and upgrade to JDK17.

@kaibocai
Copy link
Member

kaibocai commented Jun 9, 2023

@CrawX, thanks for reaching out. I draft a PR at #704 to support customized gson instance from customer. Because the team is heads-down on some other priority tasks, they haven't found time to review the PR. How about I provide you a private build which you can play with on azure portal? Thanks.

@CrawX
Copy link
Author

CrawX commented Jun 10, 2023

I'm fine with waiting a bit more, my priority was to understand if this is still being worked on and to get a rough timeline if possible, thanks for looking into it though!

@CrawX
Copy link
Author

CrawX commented Jun 28, 2023

Azure/azure-sdk-for-java/issues/30458 seems to be available now. Any update when this can be completed?

@kaibocai
Copy link
Member

Azure/azure-sdk-for-java/issues/30458 seems to be available now. Any update when this can be completed?

In the reviewing process of the PR, once it is ready will merge it. Should be done in the next one or two weeks. Thank you.

@kaibocai kaibocai linked a pull request Jun 28, 2023 that will close this issue
7 tasks
@kaibocai
Copy link
Member

kaibocai commented Jun 28, 2023

@CrawX do you have some test cases handy that I can use to validate my PR. I have run some simple tests, just want to test it more before releasing it. Thanks.

@CrawX
Copy link
Author

CrawX commented Jun 30, 2023

I don't have a simple testcase ready for this but if I recall correctly, you can just return a class containing a LocalDateTime and you will see how its serialized (also see the stackoverflow post I linked). When the LocalDateTimeAdapter I posted in the original ticket is included, it should work as expected.

If you can point me in the direction how I can give your PR a try locally using the usual ./gradlew build azureFunctionsRun, I can test it here.

@kaibocai
Copy link
Member

kaibocai commented Jul 7, 2023

@CrawX, sorry for the late reply. Please find the steps to set the new Java worker up on your local.

  1. Clone from branch https://github.com/Azure/azure-functions-java-additions/tree/kaibocai/customize-gson, this is the SPI interface that will be used in the Java language worker. Build and install it on your local Maven repo.
  2. Clone from branch https://github.com/Azure/azure-functions-java-worker/tree/kaibocai/customize-gson -- the java language worker, update the pom file to include the version of the spi library you build in step 1. Get the jar inside the target file like
image
  1. Find your core tools installed locally, replace the Java language worker jar like below, and keep the jar name as azure-functions-java-worker.jar
image
  1. In your code implement the SPI interface https://github.com/Azure/azure-functions-java-additions/blob/359d3a0c410640d390207c91a44d3890cf557364/azure-functions-java-spi/src/main/java/com/microsoft/azure/functions/spi/inject/GsonInstanceInjector.java#L5 and return your own gson instance.
  2. Run your code on local, in this case the gson instance used for serialize and deserialize will be your own gson instance.

Let me know if you have any questions. Thanks.

@CrawX
Copy link
Author

CrawX commented Sep 18, 2023

Hi @kaibocai,

thanks for taking the time to PoC this and giving me the above instructions :) I tested it locally, one thing you seem to have omitted is that you need to register the service with the ServiceLoader (follow these docs for example). Then it worked for me!

However, this specific ServiceLoader approach is not very compatible with Spring and its beans. We'd have to create the Gson object very early (way before Spring configured its Gson instance) and therefore couldn't use the one provided by Spring (and the surrounding mechanisms such as GsonBuilderCustomizer).

Is there any way this can be refactored into a Supplier<Gson> approach that is called at runtime every time a Gson instance is needed? This way, the instance could be created lazily which would allow use of the one provided by Spring?
I tested this with a quick PoC locally, this required the removal of the static gson field in RpcJsonDataSource and instead use of the supplier everywhere but that was somewhat straightforward to me.

Cheers

@kaibocai
Copy link
Member

Hi @CrawX,

Thanks for testing it out. Yes, I missed the register step in the above instructions. Sorry for that.

In the latest spring-cloud-function-adapter-azure, the spring context is initialized at https://github.com/spring-cloud/spring-cloud-function/blob/4.0.x/spring-cloud-function-adapters/spring-cloud-function-adapter-azure/src/main/java/org/springframework/cloud/function/adapter/azure/AzureFunctionInstanceInjector.java#L66, which is right before the invocation happen.
So if I understand it correctly, the configed spring Gson instance will be initialized and put into spring context at this point. However, before this, the Java language worker already uses Gson to deserialize metadata to build your function's arguments.

I believe for most customers, besides the control of the serialized process of the response of the invocation request, they also will need control of the deserialized process of how to build their functions' arguments from Grpc metadata. That's the main reason why in the proposed way, I inject the Gson instance far before the first invocation request happens. However, as the above link shows, injecting the Gson instance at such a point I don't really have a way to get the Gson bean from spring context as spring context itself hasn't been initialized yet.

I think this probably also needs some updates for spring-cloud-function-adapter-azure to make it possible. I am adding @tzolov from Spring Cloud team for any thoughts and ideas. Thank you @tzolov

Thank you.

@CrawX
Copy link
Author

CrawX commented Oct 5, 2023

Changing your logic to Supplier<GSON>-style would still allow the user to supply a default, uncustomized GSON instance before spring is initialized and switch to the one provided by spring later. I'm not sure how much sense this makes but it would be possible from a technical point of view.

I'd really love to find a solution for this...

@kaibocai
Copy link
Member

kaibocai commented Oct 9, 2023

Hi @CrawX , sorry that I am not quite getting your point, can you draft a simple PR just to explain a little more about your idea? Thank you.

@CrawX
Copy link
Author

CrawX commented Dec 5, 2023

Hi @kaibocai,

sorry for the extended delay.

I created two versions: one uses the Supplier and the other one a small interface.

The Supplier-one uses a Supplier<Gson> internally to read the Gson instance from the SPI at every call so that the SPI may change the returned instance while the function is being started.

The second one adds a tiny interface over Gson to be able to proxy the calls. This way, the instance injected by the SPI can change the actual (de)serializer behind the covers while the function is being started. You'll have to change the GsonInstanceInjector to return this interface.
The interface would look like this:

public interface GsonInstance {

  <T> T fromJson(String json, Type typeOfT) throws JsonSyntaxException;

  String toJson(Object src);
}

Both approaches allow the SPI to change the actual serializer while the function is starting. This way, a default new Gson() instance can be used during startup and can be replaced by a customized one later.

I am not sure whether this makes sense as I'm not sure if the spring context will be available before the function arguments are deserialized. Ideally, it would be possible to customize the Gson instance for both serialization and deserialization before they're being used of course.

I would very much like to see a way to get this supported when using Azure function with Spring Boot. Since 2.7 is EOL and 3.x requires JDK17, there are no good options left right now...

Regards,
Johannes

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

Successfully merging a pull request may close this issue.

4 participants