diff --git a/.palantir/revapi.yml b/.palantir/revapi.yml index d6f09d177ddf..71c39e554a4e 100644 --- a/.palantir/revapi.yml +++ b/.palantir/revapi.yml @@ -3,6 +3,41 @@ versionOverrides: acceptedBreaks: apache-iceberg-0.14.0: org.apache.iceberg:iceberg-api: + - code: "java.annotation.added" + old: "class org.apache.iceberg.catalog.Namespace" + new: "class org.apache.iceberg.catalog.Namespace" + justification: "false positive - converting to an Immutable class doesn't break\ + \ the API" + - code: "java.annotation.added" + old: "class org.apache.iceberg.catalog.TableIdentifier" + new: "class org.apache.iceberg.catalog.TableIdentifier" + justification: "false positive - converting to an Immutable class doesn't break\ + \ the API" + - code: "java.class.nowAbstract" + old: "class org.apache.iceberg.catalog.Namespace" + new: "class org.apache.iceberg.catalog.Namespace" + justification: "false positive - converting to an Immutable class doesn't break\ + \ the API" + - code: "java.class.nowAbstract" + old: "class org.apache.iceberg.catalog.TableIdentifier" + new: "class org.apache.iceberg.catalog.TableIdentifier" + justification: "false positive - converting to an Immutable class doesn't break\ + \ the API" + - code: "java.method.nowAbstract" + old: "method java.lang.String org.apache.iceberg.catalog.TableIdentifier::name()" + new: "method java.lang.String org.apache.iceberg.catalog.TableIdentifier::name()" + justification: "false positive - converting to an Immutable class doesn't break\ + \ the API" + - code: "java.method.nowAbstract" + old: "method java.lang.String[] org.apache.iceberg.catalog.Namespace::levels()" + new: "method java.lang.String[] org.apache.iceberg.catalog.Namespace::levels()" + justification: "false positive - converting to an Immutable class doesn't break\ + \ the API" + - code: "java.method.nowAbstract" + old: "method org.apache.iceberg.catalog.Namespace org.apache.iceberg.catalog.TableIdentifier::namespace()" + new: "method org.apache.iceberg.catalog.Namespace org.apache.iceberg.catalog.TableIdentifier::namespace()" + justification: "false positive - converting to an Immutable class doesn't break\ + \ the API" - code: "java.class.defaultSerializationChanged" old: "class org.apache.iceberg.PartitionKey" new: "class org.apache.iceberg.PartitionKey" diff --git a/api/src/main/java/org/apache/iceberg/catalog/Namespace.java b/api/src/main/java/org/apache/iceberg/catalog/Namespace.java index e66be71cfcae..92b79796a098 100644 --- a/api/src/main/java/org/apache/iceberg/catalog/Namespace.java +++ b/api/src/main/java/org/apache/iceberg/catalog/Namespace.java @@ -18,21 +18,21 @@ */ package org.apache.iceberg.catalog; -import java.util.Arrays; import java.util.function.Predicate; import java.util.regex.Pattern; import org.apache.iceberg.relocated.com.google.common.base.Joiner; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.immutables.value.Value; /** A namespace in a {@link Catalog}. */ -public class Namespace { - private static final Namespace EMPTY_NAMESPACE = new Namespace(new String[] {}); +@Value.Immutable +public abstract class Namespace { private static final Joiner DOT = Joiner.on('.'); private static final Predicate CONTAINS_NULL_CHARACTER = Pattern.compile("\u0000", Pattern.UNICODE_CHARACTER_CLASS).asPredicate(); public static Namespace empty() { - return EMPTY_NAMESPACE; + return ImmutableNamespace.builder().levels(new String[] {}).build(); } public static Namespace of(String... levels) { @@ -48,52 +48,25 @@ public static Namespace of(String... levels) { "Cannot create a namespace with the null-byte character"); } - return new Namespace(levels); + return ImmutableNamespace.builder().levels(levels).build(); } - private final String[] levels; - - private Namespace(String[] levels) { - this.levels = levels; - } - - public String[] levels() { - return levels; - } + public abstract String[] levels(); public String level(int pos) { - return levels[pos]; + return levels()[pos]; } public boolean isEmpty() { - return levels.length == 0; + return levels().length == 0; } public int length() { - return levels.length; - } - - @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - - if (other == null || getClass() != other.getClass()) { - return false; - } - - Namespace namespace = (Namespace) other; - return Arrays.equals(levels, namespace.levels); - } - - @Override - public int hashCode() { - return Arrays.hashCode(levels); + return levels().length; } @Override public String toString() { - return DOT.join(levels); + return DOT.join(levels()); } } diff --git a/api/src/main/java/org/apache/iceberg/catalog/TableIdentifier.java b/api/src/main/java/org/apache/iceberg/catalog/TableIdentifier.java index 8531fc63615a..bf70dce056dc 100644 --- a/api/src/main/java/org/apache/iceberg/catalog/TableIdentifier.java +++ b/api/src/main/java/org/apache/iceberg/catalog/TableIdentifier.java @@ -19,29 +19,35 @@ package org.apache.iceberg.catalog; import java.util.Arrays; -import java.util.Objects; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.base.Splitter; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; +import org.immutables.value.Value; /** Identifies a table in iceberg catalog. */ -public class TableIdentifier { +@Value.Immutable +public abstract class TableIdentifier { private static final Splitter DOT = Splitter.on('.'); - private final Namespace namespace; - private final String name; + /** Returns the identifier namespace. */ + public abstract Namespace namespace(); + + /** Returns the identifier name. */ + public abstract String name(); public static TableIdentifier of(String... names) { Preconditions.checkArgument(names != null, "Cannot create table identifier from null array"); Preconditions.checkArgument( names.length > 0, "Cannot create table identifier without a table name"); - return new TableIdentifier( - Namespace.of(Arrays.copyOf(names, names.length - 1)), names[names.length - 1]); + return ImmutableTableIdentifier.builder() + .namespace(Namespace.of(Arrays.copyOf(names, names.length - 1))) + .name(names[names.length - 1]) + .build(); } public static TableIdentifier of(Namespace namespace, String name) { - return new TableIdentifier(namespace, name); + return ImmutableTableIdentifier.builder().namespace(namespace).name(name).build(); } public static TableIdentifier parse(String identifier) { @@ -50,12 +56,9 @@ public static TableIdentifier parse(String identifier) { return TableIdentifier.of(Iterables.toArray(parts, String.class)); } - private TableIdentifier(Namespace namespace, String name) { - Preconditions.checkArgument( - name != null && !name.isEmpty(), "Invalid table name: null or empty"); - Preconditions.checkArgument(namespace != null, "Invalid Namespace: null"); - this.namespace = namespace; - this.name = name; + @Value.Check + protected void check() { + Preconditions.checkArgument(!name().isEmpty(), "Invalid table name: empty"); } /** @@ -64,17 +67,7 @@ private TableIdentifier(Namespace namespace, String name) { * @return true if the namespace is not empty, false otherwise */ public boolean hasNamespace() { - return !namespace.isEmpty(); - } - - /** Returns the identifier namespace. */ - public Namespace namespace() { - return namespace; - } - - /** Returns the identifier name. */ - public String name() { - return name; + return !namespace().isEmpty(); } public TableIdentifier toLowerCase() { @@ -84,31 +77,12 @@ public TableIdentifier toLowerCase() { return TableIdentifier.of(Namespace.of(newLevels), newName); } - @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - - if (other == null || getClass() != other.getClass()) { - return false; - } - - TableIdentifier that = (TableIdentifier) other; - return namespace.equals(that.namespace) && name.equals(that.name); - } - - @Override - public int hashCode() { - return Objects.hash(namespace, name); - } - @Override public String toString() { if (hasNamespace()) { - return namespace.toString() + "." + name; + return namespace().toString() + "." + name(); } else { - return name; + return name(); } } } diff --git a/api/src/test/java/org/apache/iceberg/catalog/TestTableIdentifier.java b/api/src/test/java/org/apache/iceberg/catalog/TestTableIdentifier.java index 61eb41aa3460..cb9c5de58862 100644 --- a/api/src/test/java/org/apache/iceberg/catalog/TestTableIdentifier.java +++ b/api/src/test/java/org/apache/iceberg/catalog/TestTableIdentifier.java @@ -56,11 +56,11 @@ public void testToLowerCase() { public void testInvalidTableName() { Assertions.assertThatThrownBy(() -> TableIdentifier.of(Namespace.empty(), "")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid table name: null or empty"); + .hasMessage("Invalid table name: empty"); Assertions.assertThatThrownBy(() -> TableIdentifier.of(Namespace.empty(), null)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid table name: null or empty"); + .isInstanceOf(NullPointerException.class) + .hasMessage("name"); } @Test @@ -74,7 +74,7 @@ public void testNulls() { .hasMessage("Cannot parse table identifier: null"); Assertions.assertThatThrownBy(() -> TableIdentifier.of(null, "name")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid Namespace: null"); + .isInstanceOf(NullPointerException.class) + .hasMessage("namespace"); } }