caching CSV schemas with view (#297)

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<JavaType,CsvSchema> _untypedSchemas;
+    protected final LRUMap<ViewKey,CsvSchema> _untypedSchemas;
 
     /**
      * Simple caching for schema instances, given that they are relatively expensive
      * to construct; this one is for typed schemas
      */
-    protected final LRUMap<JavaType,CsvSchema> _typedSchemas;
+    protected final LRUMap<ViewKey,CsvSchema> _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<JavaType,CsvSchema>(8,32);
-        _typedSchemas = new LRUMap<JavaType,CsvSchema>(8,32);
+        _untypedSchemas = new LRUMap<ViewKey,CsvSchema>(8,32);
+        _typedSchemas = new LRUMap<ViewKey,CsvSchema>(8,32);
     }
 
     /**
@@ -128,8 +174,8 @@ public CsvMapper(CsvFactory f)
     protected CsvMapper(CsvMapper src)
     {
         super(src);
-        _untypedSchemas = new LRUMap<JavaType,CsvSchema>(8,32);
-        _typedSchemas = new LRUMap<JavaType,CsvSchema>(8,32);
+        _untypedSchemas = new LRUMap<ViewKey,CsvSchema>(8,32);
+        _typedSchemas = new LRUMap<ViewKey,CsvSchema>(8,32);
     }
 
     /**
@@ -422,34 +468,28 @@ public final CsvSchema typedSchemaForWithView(TypeReference<?> pojoTypeRef, Clas
     /**********************************************************************
      */
 
-    protected CsvSchema _schemaFor(JavaType pojoType, LRUMap<JavaType,CsvSchema> schemas,
+    protected CsvSchema _schemaFor(JavaType pojoType, LRUMap<ViewKey,CsvSchema> 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<JavaType,CsvSchema> schemas, boolean typed) {
+    protected CsvSchema _schemaFor(JavaType pojoType, LRUMap<ViewKey,CsvSchema> schemas, boolean typed) {
         return _schemaFor(pojoType, schemas, typed, null);
     }
 

csv/src/test/java/com/fasterxml/jackson/dataformat/csv/schema/SchemaCaching288Test.java → 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,17 +10,15 @@
 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
     {
         @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());
+
+
     }
 }