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

CsvMapper.typedSchemaForWithView() with DEFAULT_VIEW_INCLUSION #308

Closed
mrpiggi opened this issue Feb 10, 2022 · 4 comments
Closed

CsvMapper.typedSchemaForWithView() with DEFAULT_VIEW_INCLUSION #308

mrpiggi opened this issue Feb 10, 2022 · 4 comments
Labels
Milestone

Comments

@mrpiggi
Copy link
Contributor

mrpiggi commented Feb 10, 2022

Using CsvMapper.typedSchemaForWithView(Class<?> pojoType, Class<?> view) for a pojoType—which does not define a default view with @JsonView as type annotation—ignores fields without an @JsonView annotation during the creation of the schema. This behavior differs in comparison to JsonMapper. See the given test case.

package com.fasterxml.jackson.dataformat.csv;

import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Collectors;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonView;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.json.JsonMapper;

import org.junit.Test;


public class SchemaDefaultView308Test extends ModuleTestBase {

	@Test
	public void testJsonVsCsv() throws JsonProcessingException {

		final ObjectMapper jsonMapperDisabled = JsonMapper.builder().disable(MapperFeature.DEFAULT_VIEW_INCLUSION).build();
		final CsvMapper csvMapperDisabled = CsvMapper.builder().disable(MapperFeature.DEFAULT_VIEW_INCLUSION).build();

		final ObjectMapper jsonMapperEnabled = JsonMapper.builder().enable(MapperFeature.DEFAULT_VIEW_INCLUSION).build();
		final CsvMapper csvMapperEnabled = CsvMapper.builder().enable(MapperFeature.DEFAULT_VIEW_INCLUSION).build();

		final ExplicitDefaultViewPojo explicit = new ExplicitDefaultViewPojo(true, "text", 1234);
		final ImplicitDefaultViewPojo implicit = new ImplicitDefaultViewPojo(true, "text", 1234);


		compareExpected(jsonMapperDisabled, csvMapperDisabled, explicit, null, "flag", "text", "number");
		compareExpected(jsonMapperDisabled, csvMapperDisabled, explicit, DefaultView.class, "flag");
		compareExpected(jsonMapperDisabled, csvMapperDisabled, explicit, BaseView.class, "flag", "text");
		compareExpected(jsonMapperDisabled, csvMapperDisabled, explicit, ExtendedView.class, "flag", "text", "number");

		compareExpected(jsonMapperDisabled, csvMapperDisabled, implicit, null, "flag", "text", "number");
		compareExpected(jsonMapperDisabled, csvMapperDisabled, implicit, DefaultView.class);
		compareExpected(jsonMapperDisabled, csvMapperDisabled, implicit, BaseView.class, "text");
		compareExpected(jsonMapperDisabled, csvMapperDisabled, implicit, ExtendedView.class, "text", "number");

		compareExpected(jsonMapperEnabled, csvMapperEnabled, explicit, null, "flag", "text", "number");
		compareExpected(jsonMapperEnabled, csvMapperEnabled, explicit, DefaultView.class, "flag");
		compareExpected(jsonMapperEnabled, csvMapperEnabled, explicit, BaseView.class, "flag", "text");
		compareExpected(jsonMapperEnabled, csvMapperEnabled, explicit, ExtendedView.class, "flag", "text", "number");

		System.err.println("serialization with 'DEFAULT_VIEW_INCLUSION' differs between JSON and CSV\n");
		compareExpected(jsonMapperEnabled, csvMapperEnabled, implicit, null, "flag", "text", "number");
		compareExpected(jsonMapperEnabled, csvMapperEnabled, implicit, DefaultView.class, "flag");
		compareExpected(jsonMapperEnabled, csvMapperEnabled, implicit, BaseView.class, "flag", "text");
		compareExpected(jsonMapperEnabled, csvMapperEnabled, implicit, ExtendedView.class, "flag", "text", "number");

	}


	private static void compareExpected(
			final ObjectMapper jsonMapper,
			final CsvMapper csvMapper,
			final Object pojo,
			final Class<?> view,
			final String... expectedNames
	) throws JsonProcessingException {

		final Set<String> actualJsonNames = new HashSet<>();
		final String json = jsonMapper.writerWithView(view).writeValueAsString(pojo);
		jsonMapper.readTree(json).fieldNames().forEachRemaining(actualJsonNames::add);
		assertEquals(Arrays.stream(expectedNames).collect(Collectors.toSet()), actualJsonNames);

		final Set<String> actualCsvNames = new HashSet<>();
		final CsvSchema schema = csvMapper.typedSchemaForWithView(pojo.getClass(), view);
		schema.rebuild().getColumns().forEachRemaining(c -> actualCsvNames.add(c.getName()));
		assertEquals(
				view == null ? "null" : view.getSimpleName() + " misses fields/columns",
				actualJsonNames,
				actualCsvNames
		);

	}


	interface DefaultView {}
	interface BaseView extends DefaultView {}
	interface ExtendedView extends BaseView {}

	
	@JsonView(DefaultView.class)
	class ExplicitDefaultViewPojo {

		@JsonProperty
		boolean flag;
		@JsonProperty
		@JsonView(BaseView.class)
		String text;
		@JsonProperty
		@JsonView(ExtendedView.class)
		Integer number;

		ExplicitDefaultViewPojo(
				final Boolean flag,
				final String text,
				final Integer number
		) {

			this.flag = flag;
			this.text = text;
			this.number = number;

		}

	}

	
	class ImplicitDefaultViewPojo {

		@JsonProperty
		boolean flag;
		@JsonProperty
		@JsonView(BaseView.class)
		String text;
		@JsonProperty
		@JsonView(ExtendedView.class)
		Integer number;

		ImplicitDefaultViewPojo(
				final Boolean flag,
				final String text,
				final Integer number
		) {

			this.flag = flag;
			this.text = text;
			this.number = number;

		}

	}

}
@mrpiggi
Copy link
Contributor Author

mrpiggi commented Feb 11, 2022

if (!ViewMatcher.construct(views).isVisibleForView(view)) {
continue;
}

This should maybe be changed to

if (!(getSerializationConfig().isEnabled(MapperFeature.DEFAULT_VIEW_INCLUSION) || ViewMatcher.construct(views).isVisibleForView(view))) {
    continue;
}

but I didn't test it and so I do not know, if this would effect other cases.

@mrpiggi
Copy link
Contributor Author

mrpiggi commented Feb 12, 2022

Actually, it boils down to one of these three expressions

!ViewMatcher.construct(views).isVisibleForView(view) && !(views == null && this.getSerializationConfig().isEnabled(MapperFeature.DEFAULT_VIEW_INCLUSION))
!ViewMatcher.construct(views).isVisibleForView(view) && (views != null || !this.getSerializationConfig().isEnabled(MapperFeature.DEFAULT_VIEW_INCLUSION))
!(ViewMatcher.construct(views).isVisibleForView(view) || (views == null && this.getSerializationConfig().isEnabled(MapperFeature.DEFAULT_VIEW_INCLUSION)))

meaning, serialize this property when any of these two conditions is true

  • at least one annotated default view is visible for active view
  • no annotated default view present and default view inclusion is enabled

@cowtowncoder
Copy link
Member

Thank you very much for reporting this & submitting a patch. I hope to check it out over the weekend, get it merged.
Sounds like your analysis is correct but I want to check just in case.

@cowtowncoder cowtowncoder changed the title CsvMapper#typedSchemaForWithView with DEFAULT_VIEW_INCLUSION CsvMapper.typedSchemaForWithView() with DEFAULT_VIEW_INCLUSION Feb 13, 2022
cowtowncoder added a commit that referenced this issue Feb 13, 2022
@cowtowncoder cowtowncoder added this to the 2.13.2 milestone Feb 13, 2022
@cowtowncoder
Copy link
Member

Thank you again for reporting, fixing this @mrpiggi! It will be in 2.13.2 once I get that released (probably later this month).

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

No branches or pull requests

2 participants