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

Regression in Include.NON_DEFAULT set globally #1757

Open
yhack opened this issue Aug 30, 2017 · 10 comments
Open

Regression in Include.NON_DEFAULT set globally #1757

yhack opened this issue Aug 30, 2017 · 10 comments

Comments

@yhack
Copy link

yhack commented Aug 30, 2017

When I upgraded to version 2.9.0 I hit #1586. I believe this is a valid issue. My use case requires setting Include.NON_DEFAULT globally and serialize -> deserialize must result in the same object. As per #1351, Include.NON_DEFAULT has two distinct modes.

  1. When used for specific class, it will try to use default values for properties as assigned by the default no-arguments constructor: one must exist.
  2. If used on property or as global default inclusion, value to check are default values for the type (in case of scalars, value that a class member would be initialized to; for other types, same as "empty" value)

Mode 1 is how Jackson used to work when Include.NON_DEFAULT was set globally prior to version 2.8.3. It produces the objects that are the same after serialization and desarialization. Mode 2 can result in cases where serialize -> deserialize produces different objects. For my use case I need consistency (Mode 1) globally so I attempted to use the workaround suggested in #1586.

Value non_default = JsonInclude.Value.construct(Include.NON_DEFAULT, Include.NON_DEFAULT);
objectMapper.configOverride(Holder.class).setInclude(non_default);

This turned out to be too cumbersome because I have many classes including nested classes that would need to be set individually. For now I am staying with version 2.8.2, the last version before this regression was introduced.

I understand Mode 2 was difficult to implement technically. May I suggest that Mode 2 functionality be separated into a different Include enum, something like NON_DEFAULT_MEMBER?

@cowtowncoder
Copy link
Member

I think what would help here is a simple demonstration of what you mean wrt round-trip consistency.

But as to work-around, there is the global default setting, not just per-type:

ObjectMapper.setDefaultPropertyInclusion(....)

so just in case this was missing knowledge I'll add it here. This setting can be overridden by per-type config overrides, per-class annotation and per-property annotation.

@yhack
Copy link
Author

yhack commented Aug 30, 2017

Sure. Here's a simple procedure to demonstrate the round trip consistency issue using version 2.9.0:

  1. Create an ObjectMapper and set SerializationInclusion to Include.NON_DEFAULT.
  2. Create a class with a single String property called myProperty.
  3. Instantiate the class and set myProperty to an empty string.
  4. Serialize the object. It will be empty.
  5. Deserialize the object. myProperty will be null.
  6. "" != null

I didn't see any difference when I tried setDefaultPropertyInclusion.

@bwaldvogel
Copy link

bwaldvogel commented Oct 17, 2017

I also hit this issue.

Is there a known workaround in 2.9.x?

It works by overriding it per-class with

JsonInclude.Value nonDefault = JsonInclude.Value.construct(Include.NON_DEFAULT, Include.NON_DEFAULT);
mapper.configOverride(MyClass.class).setInclude(nonDefault);

However, the global default as suggested by @cowtowncoder doesn’t work for me:

mapper.setDefaultPropertyInclusion(nonDefault);

@jflefebvre06
Copy link

+1

1 similar comment
@jartysiewicz
Copy link

+1

@bwaldvogel
Copy link

@cowtowncoder: Could you give us an update how to workaround this behaviour on a global level?

@cowtowncoder
Copy link
Member

@bwaldvogel at this point a specific reproducible unit test would be needed to ensure I actually know the specific failure mode. Textual description is useful but not sufficient. This because there are at least 2 possible things that could be problematic here.

@bwaldvogel
Copy link

bwaldvogel commented Feb 9, 2018

public class JacksonIssue1757Test {

	public static class Entity {
		private String someFieldWithDefault = "a default";

		public void setSomeFieldWithDefault(String someFieldWithDefault) {
			this.someFieldWithDefault = someFieldWithDefault;
		}

		public String getSomeFieldWithDefault() {
			return someFieldWithDefault;
		}
	}

	@Test
	public void testDefaultSettings() throws Exception {
		ObjectMapper objectMapper = new ObjectMapper();

		String json = objectMapper.writer().writeValueAsString(new Entity());
		assertEquals("{\"someFieldWithDefault\":\"a default\"}", json);

		Entity entityFromJson = objectMapper.reader().forType(Entity.class).readValue(json);
		assertEquals("a default", entityFromJson.someFieldWithDefault);
	}

	@Test
	public void testIncludeNonDefault() throws Exception {
		ObjectMapper objectMapper = new ObjectMapper();
		objectMapper.setSerializationInclusion(Include.NON_DEFAULT);

		String json = objectMapper.writer().writeValueAsString(new Entity());
		assertEquals("{}", json);

		Entity entityFromJson = objectMapper.reader().forType(Entity.class).readValue(json);
		assertEquals("a default", entityFromJson.someFieldWithDefault);
	}

	@Test
        // note: not applicable for Jackson <2.9
	public void testIncludeNonDefault_GlobalConfigOverride() throws Exception {
		ObjectMapper objectMapper = new ObjectMapper();
		objectMapper.setSerializationInclusion(Include.NON_DEFAULT);
		JsonInclude.Value nonDefault = JsonInclude.Value.construct(Include.NON_DEFAULT, Include.NON_DEFAULT);
		objectMapper.setDefaultPropertyInclusion(nonDefault);

		String json = objectMapper.writer().writeValueAsString(new Entity());
		assertEquals("{}", json);

		Entity entityFromJson = objectMapper.reader().forType(Entity.class).readValue(json);
		assertEquals("a default", entityFromJson.someFieldWithDefault);
	}

	@Test
	public void testIncludeNonDefault_SpecificConfigOverride() throws Exception {
		ObjectMapper objectMapper = new ObjectMapper();
		objectMapper.setSerializationInclusion(Include.NON_DEFAULT);
		JsonInclude.Value nonDefault = JsonInclude.Value.construct(Include.NON_DEFAULT, Include.NON_DEFAULT);
		objectMapper.configOverride(Entity.class).setInclude(nonDefault);

		String json = objectMapper.writer().writeValueAsString(new Entity());
		assertEquals("{}", json);

		Entity entityFromJson = objectMapper.reader().forType(Entity.class).readValue(json);
		assertEquals("a default", entityFromJson.someFieldWithDefault);
	}

}

The tests succeed with jackson-databind up to 2.8.2.
testIncludeNonDefault starts to break since 2.8.3.
In testIncludeNonDefault_SpecificConfigOverride I’ve applied the workaround discussed in this ticket. However, the global config override (testIncludeNonDefault_GlobalConfigOverride) doesn’t work.

@cowtowncoder: Please let me know if you need more information, thanks!

@bwaldvogel
Copy link

@cowtowncoder: Is there a misunderstanding on my side or maybe something that could be improved in Jackson?

@cowtowncoder
Copy link
Member

@bwaldvogel Unfortunately I haven't had time to look into this issue, and probably will not for near future.

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

No branches or pull requests

5 participants