Aqqq A On Wed, Apr 10, 2019 at 9:10 AM <[email protected]> wrote:
> This is an automated email from the ASF dual-hosted git repository. > > blue pushed a commit to branch master > in repository https://gitbox.apache.org/repos/asf/incubator-iceberg.git > > > The following commit(s) were added to refs/heads/master by this push: > new 2c6d132 Apply Baseline to iceberg-common and fix Baseline for > iceberg-api (#152) > 2c6d132 is described below > > commit 2c6d13228810fc20a8792e40c645ea35efda9c5f > Author: mccheah <[email protected]> > AuthorDate: Wed Apr 10 09:10:08 2019 -0700 > > Apply Baseline to iceberg-common and fix Baseline for iceberg-api > (#152) > --- > .../org/apache/iceberg/io/CloseableIterable.java | 2 +- > .../org/apache/iceberg/transforms/Timestamps.java | 3 +- > .../apache/iceberg/transforms/TestTimestamps.java | 20 +++---- > build.gradle | 65 > +++++++++++++++++++--- > .../java/org/apache/iceberg/common/DynClasses.java | 10 ++-- > .../org/apache/iceberg/common/DynConstructors.java | 11 ++-- > .../java/org/apache/iceberg/common/DynFields.java | 47 ++++++++-------- > .../java/org/apache/iceberg/common/DynMethods.java | 63 > +++++++++++---------- > 8 files changed, 140 insertions(+), 81 deletions(-) > > diff --git > a/api/src/main/java/org/apache/iceberg/io/CloseableIterable.java > b/api/src/main/java/org/apache/iceberg/io/CloseableIterable.java > index 24c0198..329bd4e 100644 > --- a/api/src/main/java/org/apache/iceberg/io/CloseableIterable.java > +++ b/api/src/main/java/org/apache/iceberg/io/CloseableIterable.java > @@ -20,12 +20,12 @@ > package org.apache.iceberg.io; > > import com.google.common.base.Preconditions; > -import org.apache.iceberg.exceptions.RuntimeIOException; > import java.io.Closeable; > import java.io.IOException; > import java.util.Collections; > import java.util.Iterator; > import java.util.function.Function; > +import org.apache.iceberg.exceptions.RuntimeIOException; > > public interface CloseableIterable<T> extends Iterable<T>, Closeable { > static <E> CloseableIterable<E> withNoopClose(Iterable<E> iterable) { > diff --git > a/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java > b/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java > index 1b5fc1d..f01ea05 100644 > --- a/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java > +++ b/api/src/main/java/org/apache/iceberg/transforms/Timestamps.java > @@ -53,7 +53,8 @@ enum Timestamps implements Transform<Long, Integer> { > OffsetDateTime timestamp = Instant > .ofEpochSecond(timestampMicros / 1_000_000) > .atOffset(ZoneOffset.UTC); > - return (int) granularity.between(EPOCH, timestamp); > + Integer year = Long.valueOf(granularity.between(EPOCH, > timestamp)).intValue(); > + return year; > } > > @Override > diff --git > a/api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java > b/api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java > index 25e22a5..57aefca 100644 > --- a/api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java > +++ b/api/src/test/java/org/apache/iceberg/transforms/TestTimestamps.java > @@ -28,21 +28,21 @@ public class TestTimestamps { > @Test > public void testTimestampWithoutZoneToHumanString() { > Types.TimestampType type = Types.TimestampType.withoutZone(); > - Literal<Integer> date = > Literal.of("2017-12-01T10:12:55.038194").to(type); > + Literal<Long> date = > Literal.of("2017-12-01T10:12:55.038194").to(type); > > - Transform<Integer, Integer> year = Transforms.year(type); > + Transform<Long, Integer> year = Transforms.year(type); > Assert.assertEquals("Should produce the correct Human string", > "2017", year.toHumanString(year.apply(date.value()))); > > - Transform<Integer, Integer> month = Transforms.month(type); > + Transform<Long, Integer> month = Transforms.month(type); > Assert.assertEquals("Should produce the correct Human string", > "2017-12", month.toHumanString(month.apply(date.value()))); > > - Transform<Integer, Integer> day = Transforms.day(type); > + Transform<Long, Integer> day = Transforms.day(type); > Assert.assertEquals("Should produce the correct Human string", > "2017-12-01", day.toHumanString(day.apply(date.value()))); > > - Transform<Integer, Integer> hour = Transforms.hour(type); > + Transform<Long, Integer> hour = Transforms.hour(type); > Assert.assertEquals("Should produce the correct Human string", > "2017-12-01-10", hour.toHumanString(hour.apply(date.value()))); > } > @@ -50,22 +50,22 @@ public class TestTimestamps { > @Test > public void testTimestampWithZoneToHumanString() { > Types.TimestampType type = Types.TimestampType.withZone(); > - Literal<Integer> date = > Literal.of("2017-12-01T10:12:55.038194-08:00").to(type); > + Literal<Long> date = > Literal.of("2017-12-01T10:12:55.038194-08:00").to(type); > > - Transform<Integer, Integer> year = Transforms.year(type); > + Transform<Long, Integer> year = Transforms.year(type); > Assert.assertEquals("Should produce the correct Human string", > "2017", year.toHumanString(year.apply(date.value()))); > > - Transform<Integer, Integer> month = Transforms.month(type); > + Transform<Long, Integer> month = Transforms.month(type); > Assert.assertEquals("Should produce the correct Human string", > "2017-12", month.toHumanString(month.apply(date.value()))); > > - Transform<Integer, Integer> day = Transforms.day(type); > + Transform<Long, Integer> day = Transforms.day(type); > Assert.assertEquals("Should produce the correct Human string", > "2017-12-01", day.toHumanString(day.apply(date.value()))); > > // the hour is 18 because the value is always UTC > - Transform<Integer, Integer> hour = Transforms.hour(type); > + Transform<Long, Integer> hour = Transforms.hour(type); > Assert.assertEquals("Should produce the correct Human string", > "2017-12-01-18", hour.toHumanString(hour.apply(date.value()))); > } > diff --git a/build.gradle b/build.gradle > index 12be5b9..6ac2381 100644 > --- a/build.gradle > +++ b/build.gradle > @@ -27,7 +27,8 @@ buildscript { > classpath 'com.github.jengelman.gradle.plugins:shadow:5.0.0' > classpath 'com.netflix.nebula:gradle-aggregate-javadocs-plugin:2.2.+' > classpath 'com.netflix.nebula:nebula-publishing-plugin:5.1.5' > - classpath 'com.palantir.baseline:gradle-baseline-java:0.53.0' > + classpath 'com.palantir.baseline:gradle-baseline-java:0.55.0' > + classpath 'com.diffplug.spotless:spotless-plugin-gradle:3.14.0' > classpath 'gradle.plugin.org.inferred:gradle-processors:2.1.0' > } > } > @@ -48,6 +49,7 @@ allprojects { > } > > apply plugin: 'nebula-aggregate-javadocs' > +apply plugin: 'com.palantir.baseline-config' > > subprojects { > apply plugin: 'nebula.javadoc-jar' > @@ -104,21 +106,66 @@ subprojects { > } > } > > +// The following code is temporary to allow for incrementally migrating > projects to use Baseline. > + > +// We enable baseline-idea everywhere so that everyone can use IntelliJ > to build code against the > +// Baseline style guide. > +def baselineProjects = [ project("iceberg-api"), > project("iceberg-common") ] > + > +configure(subprojects - baselineProjects) { > + // error-prone is brought in with baseline-idea, but we're not prepared > to handle error-prone > + // linting errors everywhere yet. > + pluginManager.withPlugin("com.palantir.baseline-error-prone") { > + tasks.withType(JavaCompile).configureEach { task -> > + options.errorprone.enabled = false > + } > + } > +} > + > +configure(baselineProjects) { > + // Currently, if any subproject applies the blanket Baseline plugin, it > forces the Baseline plugin > + // to be applied to ALL projects. And we are not prepared to address > all of the build errors that > + // occur as a result at this time. Furthermore, baseline-format will > not work out of the box for > + // us - see below. > + > + // Thus we concede to applying all of the Baseline plugins individually > on all the projects we are > + // ready to enforce linting on. > + apply plugin: 'org.inferred.processors' > + apply plugin: 'com.palantir.baseline-checkstyle' > + apply plugin: 'com.palantir.baseline-error-prone' > + apply plugin: 'com.palantir.baseline-scalastyle' > + apply plugin: 'com.palantir.baseline-class-uniqueness' > + apply plugin: 'com.palantir.baseline-reproducibility' > + apply plugin: 'com.palantir.baseline-exact-dependencies' > + apply plugin: 'com.palantir.baseline-release-compatibility' > + > + // Can't use the built-in Baseline spotless format because it's > opinionated about the import > + // order of having static imports after non-static imports, and this > cannot be overridden. > + > + // So we apply Spotless manually to get a similar effect to > baseline-format, but change the > + // import order. > + pluginManager.withPlugin('com.diffplug.gradle.spotless') { > + spotless { > + java { > + target 'src/main/java/**/*.java', 'src/main/test/**/*.java' > + removeUnusedImports() > + importOrder(['', 'static ']) > + trimTrailingWhitespace() > + indentWithSpaces 2 > + endWithNewline() > + } > + } > + } > +} > + > project(':iceberg-api') { > dependencies { > testCompile "org.apache.avro:avro:$avroVersion" > testCompile 'joda-time:joda-time:2.9.9' > } > - apply plugin: 'org.inferred.processors' > - pluginManager.withPlugin('com.palantir.baseline') { > - // Disable Baseline's Spotless Java checks for now. It does not > support having the static > - // imports being at the bottom of the import list. > - tasks.spotlessJava.enabled = false > - } > } > > -project(':iceberg-common') { > -} > +project(':iceberg-common') {} > > project(':iceberg-core') { > dependencies { > diff --git > a/common/src/main/java/org/apache/iceberg/common/DynClasses.java > b/common/src/main/java/org/apache/iceberg/common/DynClasses.java > index 2b03a29..d6fca62 100644 > --- a/common/src/main/java/org/apache/iceberg/common/DynClasses.java > +++ b/common/src/main/java/org/apache/iceberg/common/DynClasses.java > @@ -24,6 +24,9 @@ import java.util.LinkedHashSet; > import java.util.Set; > > public class DynClasses { > + > + private DynClasses() {} > + > public static Builder builder() { > return new Builder(); > } > @@ -39,11 +42,11 @@ public class DynClasses { > * <p> > * If not set, the current thread's ClassLoader is used. > * > - * @param loader a ClassLoader > + * @param newLoader a ClassLoader > * @return this Builder for method chaining > */ > - public Builder loader(ClassLoader loader) { > - this.loader = loader; > + public Builder loader(ClassLoader newLoader) { > + this.loader = newLoader; > return this; > } > > @@ -115,4 +118,3 @@ public class DynClasses { > } > } > } > - > diff --git > a/common/src/main/java/org/apache/iceberg/common/DynConstructors.java > b/common/src/main/java/org/apache/iceberg/common/DynConstructors.java > index 660105c..d564249 100644 > --- a/common/src/main/java/org/apache/iceberg/common/DynConstructors.java > +++ b/common/src/main/java/org/apache/iceberg/common/DynConstructors.java > @@ -34,6 +34,9 @@ import java.util.Map; > * Copied from parquet-common > */ > public class DynConstructors { > + > + private DynConstructors() {} > + > public static class Ctor<C> extends DynMethods.UnboundMethod { > private final Constructor<C> ctor; > private final Class<? extends C> constructed; > @@ -133,11 +136,11 @@ public class DynConstructors { > * <p> > * If not set, the current thread's ClassLoader is used. > * > - * @param loader a ClassLoader > + * @param newLoader a ClassLoader > * @return this Builder for method chaining > */ > - public Builder loader(ClassLoader loader) { > - this.loader = loader; > + public Builder loader(ClassLoader newLoader) { > + this.loader = newLoader; > return this; > } > > @@ -236,7 +239,7 @@ public class DynConstructors { > private static class MakeAccessible implements PrivilegedAction<Void> { > private Constructor<?> hidden; > > - public MakeAccessible(Constructor<?> hidden) { > + MakeAccessible(Constructor<?> hidden) { > this.hidden = hidden; > } > > diff --git a/common/src/main/java/org/apache/iceberg/common/DynFields.java > b/common/src/main/java/org/apache/iceberg/common/DynFields.java > index e2fedbe..f989f20 100644 > --- a/common/src/main/java/org/apache/iceberg/common/DynFields.java > +++ b/common/src/main/java/org/apache/iceberg/common/DynFields.java > @@ -33,6 +33,8 @@ import java.util.Set; > > public class DynFields { > > + private DynFields() {} > + > /** > * Convenience wrapper class around {@link java.lang.reflect.Field}. > * > @@ -84,10 +86,11 @@ public class DynFields { > */ > public BoundField<T> bind(Object target) { > Preconditions.checkState(!isStatic() || this == AlwaysNull.INSTANCE, > - "Cannot bind static field " + name); > + "Cannot bind static field %s", name); > Preconditions.checkArgument( > field.getDeclaringClass().isAssignableFrom(target.getClass()), > - "Cannot bind field " + name + " to instance of " + > + "Cannot bind field %s to instance of %s", > + name, > target.getClass()); > > return new BoundField<>(this, target); > @@ -100,7 +103,7 @@ public class DynFields { > * @throws IllegalStateException if the method is not static > */ > public StaticField<T> asStatic() { > - Preconditions.checkState(isStatic(), "Field " + name + " is not > static"); > + Preconditions.checkState(isStatic(), "Field %s is not static", > name); > return new StaticField<>(this); > } > > @@ -201,11 +204,11 @@ public class DynFields { > * <p> > * If not set, the current thread's ClassLoader is used. > * > - * @param loader a ClassLoader > + * @param newLoader a ClassLoader > * @return this Builder for method chaining > */ > - public Builder loader(ClassLoader loader) { > - this.loader = loader; > + public Builder loader(ClassLoader newLoader) { > + this.loader = newLoader; > return this; > } > > @@ -342,6 +345,21 @@ public class DynFields { > } > > /** > + * Returns the first valid implementation as a BoundMethod or throws a > + * NoSuchMethodException if there is none. > + * > + * @param target an Object on which to get and set the field > + * @param <T> Java class stored in the field > + * @return a {@link BoundField} with a valid implementation and target > + * @throws IllegalStateException if the method is static > + * @throws IllegalArgumentException if the receiver's class is > incompatible > + * @throws NoSuchFieldException if no implementation was found > + */ > + public <T> BoundField<T> buildChecked(Object target) throws > NoSuchFieldException { > + return this.<T>buildChecked().bind(target); > + } > + > + /** > * Returns the first valid implementation as a UnboundField or throws > a > * NoSuchFieldException if there is none. > * > @@ -363,21 +381,6 @@ public class DynFields { > > /** > * Returns the first valid implementation as a BoundMethod or throws a > - * NoSuchMethodException if there is none. > - * > - * @param target an Object on which to get and set the field > - * @param <T> Java class stored in the field > - * @return a {@link BoundField} with a valid implementation and target > - * @throws IllegalStateException if the method is static > - * @throws IllegalArgumentException if the receiver's class is > incompatible > - * @throws NoSuchFieldException if no implementation was found > - */ > - public <T> BoundField<T> buildChecked(Object target) throws > NoSuchFieldException { > - return this.<T>buildChecked().bind(target); > - } > - > - /** > - * Returns the first valid implementation as a BoundMethod or throws a > * RuntimeException if there is none. > * > * @param target an Object on which to get and set the field > @@ -422,7 +425,7 @@ public class DynFields { > private static class MakeFieldAccessible implements > PrivilegedAction<Void> { > private Field hidden; > > - public MakeFieldAccessible(Field hidden) { > + MakeFieldAccessible(Field hidden) { > this.hidden = hidden; > } > > diff --git > a/common/src/main/java/org/apache/iceberg/common/DynMethods.java > b/common/src/main/java/org/apache/iceberg/common/DynMethods.java > index 0a64e20..f9ece64 100644 > --- a/common/src/main/java/org/apache/iceberg/common/DynMethods.java > +++ b/common/src/main/java/org/apache/iceberg/common/DynMethods.java > @@ -34,6 +34,8 @@ import java.util.Arrays; > */ > public class DynMethods { > > + private DynMethods() {} > + > /** > * Convenience wrapper class around {@link java.lang.reflect.Method}. > * > @@ -88,10 +90,11 @@ public class DynMethods { > */ > public BoundMethod bind(Object receiver) { > Preconditions.checkState(!isStatic(), > - "Cannot bind static method " + method.toGenericString()); > + "Cannot bind static method %s", method.toGenericString()); > Preconditions.checkArgument( > > method.getDeclaringClass().isAssignableFrom(receiver.getClass()), > - "Cannot bind " + method.toGenericString() + " to instance of " + > + "Cannot bind %s to instance of %s", > + method.toGenericString(), > receiver.getClass()); > > return new BoundMethod(this, receiver); > @@ -124,14 +127,14 @@ public class DynMethods { > > @Override > public String toString() { > - return "DynMethods.UnboundMethod(name=" + name +" method=" + > + return "DynMethods.UnboundMethod(name=" + name + " method=" + > method.toGenericString() + ")"; > } > > /** > * Singleton {@link UnboundMethod}, performs no operation and returns > null. > */ > - private static UnboundMethod NOOP = new UnboundMethod(null, "NOOP") { > + private static final UnboundMethod NOOP = new UnboundMethod(null, > "NOOP") { > @Override > public <R> R invokeChecked(Object target, Object... args) throws > Exception { > return null; > @@ -217,11 +220,11 @@ public class DynMethods { > * <p> > * If not set, the current thread's ClassLoader is used. > * > - * @param loader a ClassLoader > + * @param newLoader a ClassLoader > * @return this Builder for method chaining > */ > - public Builder loader(ClassLoader loader) { > - this.loader = loader; > + public Builder loader(ClassLoader newLoader) { > + this.loader = newLoader; > return this; > } > > @@ -438,21 +441,6 @@ public class DynMethods { > > /** > * Returns the first valid implementation as a UnboundMethod or > throws a > - * NoSuchMethodException if there is none. > - * > - * @return a {@link UnboundMethod} with a valid implementation > - * @throws NoSuchMethodException if no implementation was found > - */ > - public UnboundMethod buildChecked() throws NoSuchMethodException { > - if (method != null) { > - return method; > - } else { > - throw new NoSuchMethodException("Cannot find method: " + name); > - } > - } > - > - /** > - * Returns the first valid implementation as a UnboundMethod or > throws a > * RuntimeError if there is none. > * > * @return a {@link UnboundMethod} with a valid implementation > @@ -468,30 +456,45 @@ public class DynMethods { > > /** > * Returns the first valid implementation as a BoundMethod or throws a > - * NoSuchMethodException if there is none. > + * RuntimeError if there is none. > * > * @param receiver an Object to receive the method invocation > * @return a {@link BoundMethod} with a valid implementation and > receiver > * @throws IllegalStateException if the method is static > * @throws IllegalArgumentException if the receiver's class is > incompatible > + * @throws RuntimeException if no implementation was found > + */ > + public BoundMethod build(Object receiver) { > + return build().bind(receiver); > + } > + > + /** > + * Returns the first valid implementation as a UnboundMethod or > throws a > + * NoSuchMethodException if there is none. > + * > + * @return a {@link UnboundMethod} with a valid implementation > * @throws NoSuchMethodException if no implementation was found > */ > - public BoundMethod buildChecked(Object receiver) throws > NoSuchMethodException { > - return buildChecked().bind(receiver); > + public UnboundMethod buildChecked() throws NoSuchMethodException { > + if (method != null) { > + return method; > + } else { > + throw new NoSuchMethodException("Cannot find method: " + name); > + } > } > > /** > * Returns the first valid implementation as a BoundMethod or throws a > - * RuntimeError if there is none. > + * NoSuchMethodException if there is none. > * > * @param receiver an Object to receive the method invocation > * @return a {@link BoundMethod} with a valid implementation and > receiver > * @throws IllegalStateException if the method is static > * @throws IllegalArgumentException if the receiver's class is > incompatible > - * @throws RuntimeException if no implementation was found > + * @throws NoSuchMethodException if no implementation was found > */ > - public BoundMethod build(Object receiver) { > - return build().bind(receiver); > + public BoundMethod buildChecked(Object receiver) throws > NoSuchMethodException { > + return buildChecked().bind(receiver); > } > > /** > @@ -523,7 +526,7 @@ public class DynMethods { > private static class MakeAccessible implements PrivilegedAction<Void> { > private Method hidden; > > - public MakeAccessible(Method hidden) { > + MakeAccessible(Method hidden) { > this.hidden = hidden; > } > > >
