From 8072e8c7aa171701bb47dfbf969cccf5496c339c Mon Sep 17 00:00:00 2001 From: mrpiggi Date: Fri, 17 Dec 2021 10:30:43 +0100 Subject: [PATCH] caching CSV schemas with view - inner class CsvMapper.ViewKey as cache key --- .../jackson/dataformat/csv/CsvMapper.java | 82 ++++++++++++++----- .../{schema => }/SchemaCaching288Test.java | 34 ++++++-- 2 files changed, 89 insertions(+), 27 deletions(-) rename csv/src/test/java/com/fasterxml/jackson/dataformat/csv/{schema => }/SchemaCaching288Test.java (59%) diff --git a/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvMapper.java b/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvMapper.java index 168e0e92..49af2e37 100644 --- a/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvMapper.java +++ b/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/CsvMapper.java @@ -1,6 +1,7 @@ package com.fasterxml.jackson.dataformat.csv; import java.util.Collection; +import java.util.Objects; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.*; @@ -86,17 +87,62 @@ public Builder configure(CsvGenerator.Feature f, boolean state) } } + + /** + * Simple class in order to create a map key based on {@link JavaType} and a given view. + * Used for caching associated schemas in {@code _untypedSchemas} and {@code _typedSchemas}. + */ + public static final class ViewKey + implements java.io.Serializable + { + private static final long serialVersionUID = 1L; + + private final JavaType _pojoType; + private final Class _view; + private final int _hashCode; + + + public ViewKey(final JavaType pojoType, final Class view) + { + _pojoType = pojoType; + _view = view; + _hashCode = Objects.hash(pojoType, view); + } + + + @Override + public int hashCode() { return _hashCode; } + + @Override + public boolean equals(final Object o) + { + if (o == this) { return true; } + if (o == null || o.getClass() != getClass()) { return false; } + final ViewKey other = (ViewKey) o; + if (_hashCode != other._hashCode || _view != other._view) { return false; } + return Objects.equals(_pojoType, other._pojoType); + } + + @Override + public String toString() + { + String viewName = _view != null ? _view.getName() : null; + return "[ViewKey: pojoType=" + _pojoType + ", view=" + viewName + "]"; + } + } + + /** * Simple caching for schema instances, given that they are relatively expensive * to construct; this one is for "loose" (non-typed) schemas */ - protected final LRUMap _untypedSchemas; + protected final LRUMap _untypedSchemas; /** * Simple caching for schema instances, given that they are relatively expensive * to construct; this one is for typed schemas */ - protected final LRUMap _typedSchemas; + protected final LRUMap _typedSchemas; /* /********************************************************************** @@ -114,8 +160,8 @@ public CsvMapper(CsvFactory f) super(f); // As per #11: default to alphabetic ordering enable(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY); - _untypedSchemas = new LRUMap(8,32); - _typedSchemas = new LRUMap(8,32); + _untypedSchemas = new LRUMap(8,32); + _typedSchemas = new LRUMap(8,32); } /** @@ -128,8 +174,8 @@ public CsvMapper(CsvFactory f) protected CsvMapper(CsvMapper src) { super(src); - _untypedSchemas = new LRUMap(8,32); - _typedSchemas = new LRUMap(8,32); + _untypedSchemas = new LRUMap(8,32); + _typedSchemas = new LRUMap(8,32); } /** @@ -422,34 +468,28 @@ public final CsvSchema typedSchemaForWithView(TypeReference pojoTypeRef, Clas /********************************************************************** */ - protected CsvSchema _schemaFor(JavaType pojoType, LRUMap schemas, + protected CsvSchema _schemaFor(JavaType pojoType, LRUMap schemas, boolean typed, Class view) { - // 15-Dec-2021, tatu: [dataformats-text#288] Only cache if we don't have - // a view, to avoid conflicts. For now. May be improved by changing cache - // key if that is considered a performance problem. - if (view == null) { - synchronized (schemas) { - CsvSchema s = schemas.get(pojoType); - if (s != null) { - return s; - } + final ViewKey viewKey = new ViewKey(pojoType, view); + synchronized (schemas) { + CsvSchema s = schemas.get(viewKey); + if (s != null) { + return s; } } final AnnotationIntrospector intr = _deserializationConfig.getAnnotationIntrospector(); CsvSchema.Builder builder = CsvSchema.builder(); _addSchemaProperties(builder, intr, typed, pojoType, null, view); CsvSchema result = builder.build(); - if (view == null) { // only cache without view (see above) - synchronized (schemas) { - schemas.put(pojoType, result); - } + synchronized (schemas) { + schemas.put(viewKey, result); } return result; } @Deprecated // since 2.11 (remove from 3.0 at latest) - protected CsvSchema _schemaFor(JavaType pojoType, LRUMap schemas, boolean typed) { + protected CsvSchema _schemaFor(JavaType pojoType, LRUMap schemas, boolean typed) { return _schemaFor(pojoType, schemas, typed, null); } diff --git a/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/SchemaCaching288Test.java b/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/SchemaCaching288Test.java similarity index 59% rename from csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/SchemaCaching288Test.java rename to csv/src/test/java/com/fasterxml/jackson/dataformat/csv/SchemaCaching288Test.java index 95b55984..5dd1fb1c 100644 --- a/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/SchemaCaching288Test.java +++ b/csv/src/test/java/com/fasterxml/jackson/dataformat/csv/SchemaCaching288Test.java @@ -1,4 +1,4 @@ -package com.fasterxml.jackson.dataformat.csv.schema; +package com.fasterxml.jackson.dataformat.csv; import com.fasterxml.jackson.annotation.JsonPropertyOrder; import com.fasterxml.jackson.annotation.JsonView; @@ -10,9 +10,7 @@ public class SchemaCaching288Test extends ModuleTestBase { static class ViewA { } - static class ViewAA extends ViewA { } static class ViewB { } - static class ViewBB extends ViewB { } @JsonPropertyOrder({ "a", "aa", "b" }) static class Bean288 @@ -20,7 +18,7 @@ static class Bean288 @JsonView({ ViewA.class, ViewB.class }) public String a = "1"; - @JsonView({ViewAA.class }) + @JsonView({ViewA.class }) public String aa = "2"; @JsonView(ViewB.class) @@ -40,21 +38,45 @@ public void testCachingNoViewFirst() throws Exception CsvSchema schemaNoView = mapper1.schemaFor(Bean288.class); assertEquals("1,2,3", mapper1.writer(schemaNoView).writeValueAsString(new Bean288()).trim()); + assertEquals(1, mapper1._untypedSchemas.size()); CsvSchema schemaB = mapper1.schemaForWithView(Bean288.class, ViewB.class); assertEquals("1,3", mapper1.writer(schemaB).withView(ViewB.class) .writeValueAsString(new Bean288()).trim()); + assertEquals(2, mapper1._untypedSchemas.size()); + + // check hash + mapper1.schemaFor(Bean288.class); + assertEquals(2, mapper1._untypedSchemas.size()); + mapper1.schemaForWithView(Bean288.class, ViewA.class); + assertEquals(3, mapper1._untypedSchemas.size()); + mapper1.schemaForWithView(Bean288.class, ViewB.class); + assertEquals(3, mapper1._untypedSchemas.size()); + } // [dataformats-text#288]: caching should not overlap with View public void testCachingWithViewFirst() throws Exception { CsvMapper mapper1 = mapperForCsv(); - CsvSchema schemaB = mapper1.schemaForWithView(Bean288.class, ViewB.class); - assertEquals("1,3", mapper1.writer(schemaB).withView(ViewB.class) + CsvSchema schemaA = mapper1.schemaForWithView(Bean288.class, ViewA.class); + assertEquals("1,2", mapper1.writer(schemaA).withView(ViewA.class) .writeValueAsString(new Bean288()).trim()); + assertEquals(1, mapper1._untypedSchemas.size()); + CsvSchema schemaNoView = mapper1.schemaFor(Bean288.class); assertEquals("1,2,3", mapper1.writer(schemaNoView).writeValueAsString(new Bean288()).trim()); + assertEquals(2, mapper1._untypedSchemas.size()); + + // check hash + mapper1.schemaFor(Bean288.class); + assertEquals(2, mapper1._untypedSchemas.size()); + mapper1.schemaForWithView(Bean288.class, ViewA.class); + assertEquals(2, mapper1._untypedSchemas.size()); + mapper1.schemaForWithView(Bean288.class, ViewB.class); + assertEquals(3, mapper1._untypedSchemas.size()); + + } }