Fix string-to-long coercion when afterburner is enabled

changelog/@unreleased/pr-1832.v2.yml

@@ -0,0 +1,5 @@
+type: fix
+fix:
+  description: Fix string-to-long coercion when afterburner is enabled
+  links:
+  - https://github.com/palantir/conjure-java-runtime/pull/1832

conjure-java-client-verifier/src/test/resources/ignored-test-cases.jersey.yml

@@ -99,8 +99,6 @@ client:
    - 'null'
    receiveMapEnumExampleAlias:
    - 'null'
-   receiveSafeLongExample: #allowed long coercion applies to safelong
-   - '{"value":"12"}'
 
   singleHeaderService: {}
 

conjure-java-client-verifier/src/test/resources/ignored-test-cases.retrofit.yml

@@ -136,8 +136,6 @@ client:
    - 'null'
    receiveMapEnumExampleAlias:
    - 'null'
-   receiveSafeLongExample: #allowed long coercion applies to safelong
-   - '{"value":"12"}'
 
   singleHeaderService: {}
 

conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/LenientLongModule.java

@@ -0,0 +1,94 @@
+/*
+ * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.palantir.conjure.java.serialization;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.cfg.CoercionAction;
+import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
+import com.fasterxml.jackson.databind.exc.InvalidFormatException;
+import com.fasterxml.jackson.databind.module.SimpleModule;
+import com.fasterxml.jackson.datatype.jdk8.OptionalLongDeserializer;
+import com.palantir.logsafe.exceptions.SafeIoException;
+import java.io.IOException;
+import java.util.OptionalLong;
+
+/**
+ * Provides support for the {@link Long} deserialization from JSON string and numeric values regardless of
+ *
+ * <pre>MapperFeature.ALLOW_COERCION_OF_SCALARS</pre>
+ *
+ * configuration.
+ */
+final class LenientLongModule extends SimpleModule {
+
+    LenientLongModule() {
+        super("lenient long");
+        // Register to both Long.TYPE and Long.class
+        this.addDeserializer(long.class, new LongAsStringDeserializer())
+                .addDeserializer(Long.class, new LongAsStringDeserializer())
+                .addDeserializer(OptionalLong.class, new OptionalLongAsStringDeserializer());
+    }
+
+    private static final class LongAsStringDeserializer extends StdDeserializer<Long> {
+
+        private LongAsStringDeserializer() {
+            super(Long.TYPE);
+        }
+
+        @Override
+        public Long deserialize(JsonParser jsonParser, DeserializationContext _ctxt) throws IOException {
+            switch (jsonParser.currentToken()) {
+                case VALUE_NUMBER_INT:
+                    return jsonParser.getLongValue();
+                case VALUE_STRING:
+                    return parseLong(jsonParser);
+                case VALUE_NULL:
+                    return null;
+                default:
+                    throw new SafeIoException("Expected a long value");
+            }
+        }
+
+        @Override
+        public boolean isCachable() {
+            return true;
+        }
+
+        private static Long parseLong(JsonParser jsonParser) throws IOException {
+            String value = jsonParser.getValueAsString();
+            try {
+                return Long.valueOf(value);
+            } catch (NumberFormatException e) {
+                InvalidFormatException failure =
+                        new InvalidFormatException(jsonParser, "not a valid long value", value, long.class);
+                failure.initCause(e);
+                throw failure;
+            }
+        }
+    }
+
+    private static final class OptionalLongAsStringDeserializer extends OptionalLongDeserializer {
+
+        private OptionalLongAsStringDeserializer() {}
+
+        @Override
+        protected CoercionAction _checkFromStringCoercion(DeserializationContext _ctxt, String _value) {
+            return CoercionAction.TryConvert;
+        }
+    }
+}

conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java

@@ -20,15 +20,12 @@
 import com.fasterxml.jackson.databind.MapperFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.SerializationFeature;
-import com.fasterxml.jackson.databind.cfg.CoercionAction;
-import com.fasterxml.jackson.databind.cfg.CoercionInputShape;
 import com.fasterxml.jackson.dataformat.cbor.CBORFactory;
 import com.fasterxml.jackson.dataformat.smile.SmileFactory;
 import com.fasterxml.jackson.datatype.guava.GuavaModule;
 import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
 import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
 import com.fasterxml.jackson.module.afterburner.AfterburnerModule;
-import java.util.OptionalLong;
 
 public final class ObjectMappers {
 
@@ -131,14 +128,12 @@ public static ObjectMapper newSmileServerObjectMapper() {
      * </ul>
      */
     public static ObjectMapper withDefaultModules(ObjectMapper mapper) {
-        allowStringCoercion(mapper, long.class);
-        allowStringCoercion(mapper, Long.class);
-        allowStringCoercion(mapper, OptionalLong.class);
         return mapper.registerModule(new GuavaModule())
                 .registerModule(new ShimJdk7Module())
                 .registerModule(new Jdk8Module().configureAbsentsAsNulls(true))
                 .registerModule(new AfterburnerModule())
                 .registerModule(new JavaTimeModule())
+                .registerModule(new LenientLongModule())
                 .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
                 .disable(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS)
                 .disable(DeserializationFeature.ADJUST_DATES_TO_CONTEXT_TIME_ZONE)
@@ -148,10 +143,4 @@ public static ObjectMapper withDefaultModules(ObjectMapper mapper) {
                 .disable(MapperFeature.ALLOW_COERCION_OF_SCALARS)
                 .disable(DeserializationFeature.ACCEPT_FLOAT_AS_INT);
     }
-
-    private static void allowStringCoercion(ObjectMapper mapper, Class<?> clazz) {
-        mapper.coercionConfigFor(clazz)
-                .setAcceptBlankAsEmpty(false)
-                .setCoercion(CoercionInputShape.String, CoercionAction.TryConvert);
-    }
 }

conjure-java-jackson-serialization/src/test/java/com/palantir/conjure/java/serialization/ObjectMappersTest.java

@@ -19,12 +19,14 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.core.exc.InputCoercionException;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.exc.InvalidFormatException;
+import com.palantir.logsafe.Preconditions;
 import java.io.File;
 import java.io.IOException;
 import java.math.BigInteger;
@@ -38,6 +40,7 @@
 import java.time.ZonedDateTime;
 import java.util.Collections;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.OptionalLong;
@@ -128,11 +131,115 @@ public void testLongTypeDeserializationFromString() throws IOException {
         assertThat(MAPPER.readValue("\"1\"", Long.TYPE)).isEqualTo(1L);
     }
 
+    @Test
+    public void testLongBeanTypeDeserializationFromString() throws IOException {
+        assertThat(MAPPER.readValue("{\"value\":\"1\"}", LongBean.class)).isEqualTo(new LongBean(1L));
+    }
+
+    @Test
+    public void testLongBeanTypeDeserializationFromNumber() throws IOException {
+        assertThat(MAPPER.readValue("{\"value\":\"1\"}", LongBean.class)).isEqualTo(new LongBean(1L));
+    }
+
+    static final class LongBean {
+        @JsonProperty
+        private long value;
+
+        LongBean() {}
+
+        LongBean(long value) {
+            setValue(value);
+        }
+
+        public long getValue() {
+            return value;
+        }
+
+        public void setValue(long value) {
+            this.value = value;
+        }
+
+        @Override
+        public boolean equals(Object other) {
+            if (this == other) {
+                return true;
+            }
+            if (other == null || getClass() != other.getClass()) {
+                return false;
+            }
+            LongBean that = (LongBean) other;
+            return value == that.value;
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hashCode(value);
+        }
+
+        @Override
+        public String toString() {
+            return "LongBean{value=" + value + '}';
+        }
+    }
+
     @Test
     public void testOptionalLongTypeDeserializationFromString() throws IOException {
         assertThat(MAPPER.readValue("\"1\"", OptionalLong.class)).hasValue(1L);
     }
 
+    @Test
+    public void testOptionalLongBeanTypeDeserializationFromString() throws IOException {
+        assertThat(MAPPER.readValue("{\"value\":\"1\"}", OptionalLongBean.class))
+                .isEqualTo(new OptionalLongBean(OptionalLong.of(1L)));
+    }
+
+    @Test
+    public void testOptionalLongBeanTypeDeserializationFromNumber() throws IOException {
+        assertThat(MAPPER.readValue("{\"value\":1}", OptionalLongBean.class))
+                .isEqualTo(new OptionalLongBean(OptionalLong.of(1L)));
+    }
+
+    static final class OptionalLongBean {
+        @JsonProperty
+        private OptionalLong value = OptionalLong.empty();
+
+        OptionalLongBean() {}
+
+        OptionalLongBean(OptionalLong value) {
+            setValue(value);
+        }
+
+        public OptionalLong getValue() {
+            return value;
+        }
+
+        public void setValue(OptionalLong value) {
+            this.value = Preconditions.checkNotNull(value, "value");
+        }
+
+        @Override
+        public boolean equals(Object other) {
+            if (this == other) {
+                return true;
+            }
+            if (other == null || getClass() != other.getClass()) {
+                return false;
+            }
+            OptionalLongBean that = (OptionalLongBean) other;
+            return value.equals(that.value);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hashCode(value);
+        }
+
+        @Override
+        public String toString() {
+            return "OptionalLongBean{value=" + value + '}';
+        }
+    }
+
     @Test
     public void testLongDeserializationFromJsonNumber() throws IOException {
         assertThat(MAPPER.readValue("1", Long.class)).isEqualTo(1L);