Exclude non-fields from record serialization

Before the change, a record's getter-like methods would be considered
getters and would be parsed of serialized form. This is undesirable
because

- For record component `foo`, the `foo()` method should be considered
  the accessor. If `getFoo()` exists, it may return something else.
  Using it can lead to correctness problems.
- A method like `getFoo()` may not be accessor to a record component at
  all, it should not be part of serialization.
- The default behavior is different than one for regular classes, so a
  benign refactor from a class to a record changes what gets serialized
  as JSON -- the getter-like non-getters on a class are ignored (do not
  need `@JsonIgnore`) and are included on a record (unless annotated
  with `@JsonIgnore`). This changes serialized form in a hard to notice
  manner, because we ignore those additional properties during
  deserialization.

This commit fixes this. Now only the canonical accessors are considered
a getter in a record.

Alternative solution would be to rely on Jackson built-in record
support. However, it relies on private field access, and we disable
CAN_OVERRIDE_ACCESS_MODIFIERS mapper feature.

CHANGES

@@ -1,3 +1,7 @@
+246
+
+- Exclude getter-like record methods from JSON serialization
+
 245
 
 - Update bouncycastle to 1.78

json/src/main/java/io/airlift/json/RecordAutoDetectModule.java

@@ -1,12 +1,26 @@
 package io.airlift.json;
 
 import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.PropertyAccessor;
 import com.fasterxml.jackson.core.Version;
 import com.fasterxml.jackson.databind.AnnotationIntrospector;
 import com.fasterxml.jackson.databind.introspect.AnnotatedClass;
+import com.fasterxml.jackson.databind.introspect.AnnotatedField;
+import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
+import com.fasterxml.jackson.databind.introspect.AnnotatedMethod;
 import com.fasterxml.jackson.databind.introspect.VisibilityChecker;
 import com.fasterxml.jackson.databind.module.SimpleModule;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.RecordComponent;
+import java.util.Set;
+import java.util.stream.Stream;
+
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
 public class RecordAutoDetectModule
         extends SimpleModule
 {
@@ -20,12 +34,6 @@ public void setupModule(SetupContext context)
     private static class Introspector
             extends AnnotationIntrospector
     {
-        private static final VisibilityChecker.Std RECORD_VISIBILITY_CHECKER = VisibilityChecker.Std.defaultInstance()
-                .withGetterVisibility(JsonAutoDetect.Visibility.PUBLIC_ONLY)
-                .withCreatorVisibility(JsonAutoDetect.Visibility.DEFAULT)
-                .withFieldVisibility(JsonAutoDetect.Visibility.DEFAULT)
-                .withIsGetterVisibility(JsonAutoDetect.Visibility.DEFAULT);
-
         @Override
         public VisibilityChecker<?> findAutoDetectVisibility(AnnotatedClass ac, VisibilityChecker<?> checker)
         {
@@ -34,7 +42,7 @@ public VisibilityChecker<?> findAutoDetectVisibility(AnnotatedClass ac, Visibili
                 if (overrideAnnotation != null) {
                     return VisibilityChecker.Std.construct(JsonAutoDetect.Value.from(overrideAnnotation));
                 }
-                return RECORD_VISIBILITY_CHECKER;
+                return new RecordVisibilityChecker(ac.getRawType().asSubclass(Record.class));
             }
             return checker;
         }
@@ -45,4 +53,134 @@ public Version version()
             return Version.unknownVersion();
         }
     }
+
+    private static class RecordVisibilityChecker
+            implements VisibilityChecker<RecordVisibilityChecker>
+    {
+        private final Set<String> componentNames;
+
+        public RecordVisibilityChecker(Class<? extends Record> recordClass)
+        {
+            componentNames = Stream.of(recordClass.getRecordComponents())
+                    .map(RecordComponent::getName)
+                    .collect(toImmutableSet());
+        }
+
+        @Override
+        public RecordVisibilityChecker with(JsonAutoDetect annotation)
+        {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public RecordVisibilityChecker withOverrides(JsonAutoDetect.Value value)
+        {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public RecordVisibilityChecker with(JsonAutoDetect.Visibility visibility)
+        {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public RecordVisibilityChecker withVisibility(PropertyAccessor propertyAccessor, JsonAutoDetect.Visibility visibility)
+        {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public RecordVisibilityChecker withGetterVisibility(JsonAutoDetect.Visibility visibility)
+        {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public RecordVisibilityChecker withIsGetterVisibility(JsonAutoDetect.Visibility visibility)
+        {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public RecordVisibilityChecker withSetterVisibility(JsonAutoDetect.Visibility visibility)
+        {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public RecordVisibilityChecker withCreatorVisibility(JsonAutoDetect.Visibility visibility)
+        {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public RecordVisibilityChecker withFieldVisibility(JsonAutoDetect.Visibility visibility)
+        {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public boolean isGetterVisible(Method method)
+        {
+            if (!Modifier.isPublic(method.getModifiers())) {
+                return false;
+            }
+            return componentNames.contains(method.getName());
+        }
+
+        @Override
+        public boolean isGetterVisible(AnnotatedMethod annotatedMethod)
+        {
+            return isGetterVisible(annotatedMethod.getAnnotated());
+        }
+
+        @Override
+        public boolean isIsGetterVisible(Method method)
+        {
+            return false;
+        }
+
+        @Override
+        public boolean isIsGetterVisible(AnnotatedMethod annotatedMethod)
+        {
+            return false;
+        }
+
+        @Override
+        public boolean isSetterVisible(Method method)
+        {
+            return false;
+        }
+
+        @Override
+        public boolean isSetterVisible(AnnotatedMethod annotatedMethod)
+        {
+            return false;
+        }
+
+        @Override
+        public boolean isCreatorVisible(Member member)
+        {
+            return true;
+        }
+
+        @Override
+        public boolean isCreatorVisible(AnnotatedMember annotatedMember)
+        {
+            return true;
+        }
+
+        @Override
+        public boolean isFieldVisible(Field field)
+        {
+            return false;
+        }
+
+        @Override
+        public boolean isFieldVisible(AnnotatedField field)
+        {
+            return false;
+        }
+    }
 }

json/src/test/java/io/airlift/json/TestJsonCodec.java

@@ -15,6 +15,8 @@
  */
 package io.airlift.json;
 
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonPropertyOrder;
 import com.google.common.base.Strings;
 import com.google.common.reflect.TypeToken;
 import org.testng.annotations.Test;
@@ -257,4 +259,101 @@ public void testToJsonWithLengthLimitComplex()
         assertFalse(jsonCodec.toJsonWithLengthLimit(people, 10381).isPresent());
         assertTrue(jsonCodec.toJsonWithLengthLimit(people, 10382).isPresent());
     }
+
+    @Test
+    public void testRecordSerialization()
+    {
+        assertSerializationRoundTrip(
+                jsonCodec(MyRecord.class),
+                new MyRecord("my value"),
+                """
+                {
+                  "foo" : "my value"
+                }\
+                """);
+
+        assertSerializationRoundTrip(
+                JsonCodec.jsonCodec(MyRecordWithBeanLikeGetter.class),
+                new MyRecordWithBeanLikeGetter("my value"),
+                """
+                {
+                  "foo" : "my value"
+                }\
+                """);
+
+        assertSerializationRoundTrip(
+                JsonCodec.jsonCodec(MyRecordAdditionalGetter.class),
+                new MyRecordAdditionalGetter("my value", true, true),
+                """
+                {
+                  "additionalProperty" : "additional property value",
+                  "condition" : true,
+                  "foo" : "my value",
+                  "isCool" : true
+                }\
+                """);
+    }
+
+    private static <T> void assertSerializationRoundTrip(JsonCodec<T> codec, T object, String expected)
+    {
+        assertEquals(codec.toJson(object), expected);
+        assertEquals(codec.fromJson(expected), object);
+    }
+
+    public record MyRecord(String foo) {}
+
+    public record MyRecordWithBeanLikeGetter(String foo)
+    {
+        public String getFoo()
+        {
+            return foo();
+        }
+    }
+
+    @JsonPropertyOrder(alphabetic = true)
+    public record MyRecordAdditionalGetter(
+            // a basic property
+            String foo,
+            // a boolean property that has additional getter and is-getter
+            boolean condition,
+            // a boolean property named as an is-getter
+            boolean isCool)
+    {
+        // Might shadow actual getter for foo -- the foo() method
+        public String getFoo()
+        {
+            throw new UnsupportedOperationException("this method should not be called during serialization");
+        }
+
+        // Looks like a getter for "bar" property, there is no such record component
+        public String getBar()
+        {
+            return "there is no bar field in the record";
+        }
+
+        // Not a record component, but explicitly requested to be included in serialization
+        @JsonProperty
+        public String getAdditionalProperty()
+        {
+            return "additional property value";
+        }
+
+        // Looks like record-style getter for "baz" property, there is no such record component
+        public String baz()
+        {
+            throw new UnsupportedOperationException("this method should not be called during serialization");
+        }
+
+        // Might shadow actual getter for condition -- the condition() method
+        public boolean isCondition()
+        {
+            throw new UnsupportedOperationException("this method should not be called during serialization");
+        }
+
+        // Might shadow actual getter for condition -- the condition() method
+        public boolean getCondition()
+        {
+            throw new UnsupportedOperationException("this method should not be called during serialization");
+        }
+    }
 }