Conversation

bdgould

I keep getting MergeRequestEvent having a null objectAttributes field. It looks like the API actually uses the syntax: object_attributes. This seems to fix it. There could be other instances of elsewhere in the API.

I'm trying create a simple webhook trigger using a managed GitLab CE 17.5.1 server, and am getting event json like: sample.json

@jmini

How are you using this library?

Because the convention of turning underscore notation to camelCase should work out of the box, without us having to redefine it everywhere using a @JsonProperty property annotation.

This is also covered by Unit Tests here:
https://.com/gitlab4j/gitlab4j-api/blob/0a1b6e0dcc926bb9fa1fb284d12b89388f446548/src/test/java/org/gitlab4j/api/TestGitLabApiEvents.java

So I guess that you have a different jackson config (maybe a different ObjectMapper instance).

@jminijmini deleted the branch gitlab4j:main November 28, 2024 19:53
@jminijmini closed this Nov 28, 2024
@jmini

When I have deleted the 6.x branch (see #926 (comment)) this PR was closed, but this is not my intention. It can be changed to target main (which is the branch where the next 6.0.0 version is prepared).

@jminijmini reopened this Nov 29, 2024
@jminijmini changed the base branch from 6.x to main November 29, 2024 07:03
@jmini

I think the configuration is done here:

objectMapper.setPropertyNamingStrategy(PropertyNamingStrategy.SNAKE_CASE);

To configure the ObjectMapper that does the case transformation inside a Quarkus project you need to do this:

import javax.enterprise.inject.Instance;
import javax.inject.Singleton;

import org.gitlab4j.api.utils.JacksonJson;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;

import io.quarkus.jackson.ObjectMapperCustomizer;

public class ObjectMapperConfiguration {

	@Singleton
	ObjectMapper objectMapper(Instance<ObjectMapperCustomizer> customizers) {
		return new JacksonJson().getObjectMapper().enable(SerializationFeature.INDENT_OUTPUT);
	}
}

I am almost certain other frameworks have similar approaches.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not against having more information in Annotations on the class, to make it independent from a potential Jackson config.

To accept this Pull Request, work is needed:

  1. add the annotation everywhere needed (not just on few fields) --> consistency is very important
  2. modify the test so that they run without this specific Jackson config (and they have to stay green).

@bdgould

Hi @jmini , Sorry for the delay! I agree, I think the metadata would be helpful -- I was just tied up during the Thanksgiving holiday here in the U.S. and then some unrelated stuff. I can work through it a bit more here in the coming weeks!

I ran into the issue in a spring-boot application I was working through. I did quickly find the spring jackson configuration, however I ran into a somewhat related issue in that some of the event triggers for the webhook used multiple formats of Date/times... This would also be solved potentially with these style annotations. I'll take a broader swing at this!

@jmini

What a work! I will review ASAP and merge.

@bdgould

Ha, thanks @jmini. I may need another weekend or two to test a bit more. I tried a couple different things with our managed gitlab server and found a couple date formats were wrong -- I'd like to try a test a bunch more!

@jmini

@bdgould it could even be that some date in the test JSON files in the gitlab4j-models/src/test/resources/org/gitlab4j/models folder not really matches what gitlab is sending...

@bdgould

Yeah, I found a couple of slight issues like that. I'm just trying various things with my managed gitlab instance to verify so far.

@jmini

I fixed some more cases where the date is sent as 2018-07-01 by the server and where we were serializing as 2018-07-01T00:00:00Z in PR #1223

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

Successfully merging this pull request may close these issues.