[avro] branch master updated: AVRO-3014: Log a warning on ignored logicalType

[ie...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

iemejia pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/avro.git


The following commit(s) were added to refs/heads/master by this push:
     new 72654bf  AVRO-3014: Log a warning on ignored logicalType
72654bf is described below

commit 72654bf73c9eefec06001ab2d1181fbfb127922c
Author: Ryan Skraba <ry...@skraba.com>
AuthorDate: Wed Feb 3 17:44:17 2021 +0100

    AVRO-3014: Log a warning on ignored logicalType
---
 .../avro/src/main/java/org/apache/avro/Schema.java |   9 ++
 .../java/org/apache/avro/TestSchemaWarnings.java   | 109 +++++++++++++++++++++
 2 files changed, 118 insertions(+)

diff --git a/lang/java/avro/src/main/java/org/apache/avro/Schema.java b/lang/java/avro/src/main/java/org/apache/avro/Schema.java
index ff603cd..d62f2e3 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/Schema.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/Schema.java
@@ -49,6 +49,10 @@ import java.util.Set;
 import org.apache.avro.util.internal.Accessor;
 import org.apache.avro.util.internal.Accessor.FieldAccessor;
 import org.apache.avro.util.internal.JacksonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.avro.LogicalType.LOGICAL_TYPE_PROP;
 
 /**
  * An abstract data type.
@@ -106,6 +110,7 @@ public abstract class Schema extends JsonProperties implements Serializable {
   }
 
   static final JsonFactory FACTORY = new JsonFactory();
+  static final Logger LOG = LoggerFactory.getLogger(Schema.class);
   static final ObjectMapper MAPPER = new ObjectMapper(FACTORY);
 
   private static final int NO_HASHCODE = Integer.MIN_VALUE;
@@ -1684,6 +1689,10 @@ public abstract class Schema extends JsonProperties implements Serializable {
           }
           f.aliases = parseAliases(field);
           fields.add(f);
+          if (fieldSchema.getLogicalType() == null && getOptionalText(field, LOGICAL_TYPE_PROP) != null)
+            LOG.warn(
+                "Ignored the {}.{}.logicalType property (\"{}\"). It should probably be nested inside the \"type\" for the field.",
+                name, fieldName, getOptionalText(field, "logicalType"));
         }
         result.setFields(fields);
       } else if (type.equals("enum")) { // enum
diff --git a/lang/java/avro/src/test/java/org/apache/avro/TestSchemaWarnings.java b/lang/java/avro/src/test/java/org/apache/avro/TestSchemaWarnings.java
new file mode 100644
index 0000000..e14ec62
--- /dev/null
+++ b/lang/java/avro/src/test/java/org/apache/avro/TestSchemaWarnings.java
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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
+ *
+ *     https://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 org.apache.avro;
+
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
+
+import static org.apache.avro.LogicalType.LOGICAL_TYPE_PROP;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+
+public class TestSchemaWarnings {
+
+  private final static PrintStream originalErr = System.err;
+
+  /**
+   * The capturable replacement for the system err stream.
+   */
+  private final ByteArrayOutputStream capturedErr = new ByteArrayOutputStream();
+
+  @Before
+  public void setupStdErr() {
+    capturedErr.reset();
+    System.setErr(new PrintStream(capturedErr));
+  }
+
+  @AfterClass
+  public static void restoreStdErr() {
+    System.setErr(originalErr);
+  }
+
+  public String getCapturedStdErr() {
+    System.out.flush();
+    String stderr = new String(capturedErr.toByteArray(), StandardCharsets.UTF_8);
+    capturedErr.reset();
+    return stderr;
+  }
+
+  @Test
+  public void testWarnWhenTheLogicalTypeIsOnTheField() {
+    // A record with a single int field.
+    Schema s = SchemaBuilder.record("A").fields().requiredInt("a1").endRecord();
+
+    // Force reparsing the schema, and no warning should be logged.
+    s = new Schema.Parser().parse(s.toString());
+    assertThat(s.getField("a1").schema().getLogicalType(), nullValue());
+    assertThat(getCapturedStdErr(), is(""));
+
+    // Add the logical type annotation to the field (as opposed to the field schema)
+    // and parse it again. This is a common error, see AVRO-3014, AVRO-2015.
+    s.getField("a1").addProp(LOGICAL_TYPE_PROP, LogicalTypes.date().getName());
+    assertThat(s.getField("a1").schema().getLogicalType(), nullValue());
+
+    // Force reparsing the schema, and a warning should be logged.
+    s = new Schema.Parser().parse(s.toString());
+    assertThat(getCapturedStdErr(), containsString("Ignored the A.a1.logicalType property (\"date\"). It should"
+        + " probably be nested inside the \"type\" for the field."));
+    assertThat(s.getField("a1").schema().getLogicalType(), nullValue());
+
+    // Add the logical type annotation to the field schema. This doesn't change the
+    // logical type of an already parsed schema.
+    s.getField("a1").schema().addProp(LOGICAL_TYPE_PROP, LogicalTypes.date().getName());
+    assertThat(s.getField("a1").schema().getLogicalType(), nullValue());
+
+    // Force reparsing the schema. No warning should be logged, and the logical type
+    // should be applied.
+    s = new Schema.Parser().parse(s.toString());
+    assertThat(getCapturedStdErr(), is(""));
+    assertThat(s.getField("a1").schema().getLogicalType(), is(LogicalTypes.date()));
+
+  }
+
+  @Test
+  public void testWarnWhenTheLogicalTypeIsIgnored() {
+    // A record with a single int field.
+    Schema s = SchemaBuilder.record("A").fields().requiredLong("a1").endRecord();
+
+    // Add the logical type annotation to the field (as opposed to the field schema)
+    // and parse it again.
+    s.getField("a1").schema().addProp(LOGICAL_TYPE_PROP, LogicalTypes.date().getName());
+    // Force reparsing the schema. No warning should be logged, and the logical type
+    // should be applied.
+    s = new Schema.Parser().parse(s.toString());
+    assertThat(s.getField("a1").schema().getLogicalType(), nullValue());
+    assertThat(getCapturedStdErr(), containsString("Ignoring invalid logical type for name: date"));
+  }
+}