From c9f2c45209d678aa3af0d01f37201ac2cf699658 Mon Sep 17 00:00:00 2001 From: Steve Loughran Date: Thu, 1 Jun 2023 14:31:08 +0100 Subject: [PATCH] HADOOP-18755. openFile builder new optLong() methods break hbase-filesystem (#5704) This is a followup to HADOOP-18724. Open file fails with NumberFormatException for S3AFileSystem Contributed by Steve Loughran --- .../java/org/apache/hadoop/fs/FSBuilder.java | 56 ++++++++++++++----- .../hadoop/fs/impl/AbstractFSBuilderImpl.java | 16 +++--- .../hadoop/fs/store/TestFSBuilderSupport.java | 42 +++++++++++--- 3 files changed, 85 insertions(+), 29 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSBuilder.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSBuilder.java index 81a14e97be5..175df15a543 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSBuilder.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FSBuilder.java @@ -78,7 +78,9 @@ public interface FSBuilder> { * @return generic type B. * @see #opt(String, String) */ - B opt(@Nonnull String key, boolean value); + default B opt(@Nonnull String key, boolean value) { + return opt(key, Boolean.toString(value)); + } /** * Set optional int parameter for the Builder. @@ -88,7 +90,9 @@ public interface FSBuilder> { * @return generic type B. * @see #opt(String, String) */ - B opt(@Nonnull String key, int value); + default B opt(@Nonnull String key, int value) { + return optLong(key, value); + } /** * This parameter is converted to a long and passed @@ -102,7 +106,9 @@ public interface FSBuilder> { * @deprecated use {@link #optDouble(String, double)} */ @Deprecated - B opt(@Nonnull String key, float value); + default B opt(@Nonnull String key, float value) { + return optLong(key, (long) value); + } /** * Set optional long parameter for the Builder. @@ -112,7 +118,9 @@ public interface FSBuilder> { * @return generic type B. * @deprecated use {@link #optLong(String, long)} where possible. */ - B opt(@Nonnull String key, long value); + default B opt(@Nonnull String key, long value) { + return optLong(key, value); + } /** * Pass an optional double parameter for the Builder. @@ -126,7 +134,9 @@ public interface FSBuilder> { * @deprecated use {@link #optDouble(String, double)} */ @Deprecated - B opt(@Nonnull String key, double value); + default B opt(@Nonnull String key, double value) { + return optLong(key, (long) value); + } /** * Set an array of string values as optional parameter for the Builder. @@ -146,7 +156,9 @@ public interface FSBuilder> { * @return generic type B. * @see #opt(String, String) */ - B optLong(@Nonnull String key, long value); + default B optLong(@Nonnull String key, long value) { + return opt(key, Long.toString(value)); + } /** * Set optional double parameter for the Builder. @@ -156,7 +168,9 @@ public interface FSBuilder> { * @return generic type B. * @see #opt(String, String) */ - B optDouble(@Nonnull String key, double value); + default B optDouble(@Nonnull String key, double value) { + return opt(key, Double.toString(value)); + } /** * Set mandatory option to the Builder. @@ -178,7 +192,9 @@ public interface FSBuilder> { * @return generic type B. * @see #must(String, String) */ - B must(@Nonnull String key, boolean value); + default B must(@Nonnull String key, boolean value) { + return must(key, Boolean.toString(value)); + } /** * Set mandatory int option. @@ -188,7 +204,9 @@ public interface FSBuilder> { * @return generic type B. * @see #must(String, String) */ - B must(@Nonnull String key, int value); + default B must(@Nonnull String key, int value) { + return mustLong(key, value); + } /** * This parameter is converted to a long and passed @@ -201,7 +219,9 @@ public interface FSBuilder> { * @deprecated use {@link #mustDouble(String, double)} to set floating point. */ @Deprecated - B must(@Nonnull String key, float value); + default B must(@Nonnull String key, float value) { + return mustLong(key, (long) value); + } /** * Set mandatory long option. @@ -212,7 +232,9 @@ public interface FSBuilder> { * @see #must(String, String) */ @Deprecated - B must(@Nonnull String key, long value); + default B must(@Nonnull String key, long value) { + return mustLong(key, (long) value); + } /** * Set mandatory long option, despite passing in a floating @@ -224,7 +246,9 @@ public interface FSBuilder> { * @see #must(String, String) */ @Deprecated - B must(@Nonnull String key, double value); + default B must(@Nonnull String key, double value) { + return mustLong(key, (long) value); + } /** * Set a string array as mandatory option. @@ -244,7 +268,9 @@ public interface FSBuilder> { * @return generic type B. * @see #opt(String, String) */ - B mustLong(@Nonnull String key, long value); + default B mustLong(@Nonnull String key, long value) { + return must(key, Long.toString(value)); + } /** * Set mandatory double parameter for the Builder. @@ -254,7 +280,9 @@ public interface FSBuilder> { * @return generic type B. * @see #opt(String, String) */ - B mustDouble(@Nonnull String key, double value); + default B mustDouble(@Nonnull String key, double value) { + return must(key, Double.toString(value)); + } /** * Instantiate the object which was being built. diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/AbstractFSBuilderImpl.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/AbstractFSBuilderImpl.java index 47cf98f4746..108b60256ef 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/AbstractFSBuilderImpl.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/AbstractFSBuilderImpl.java @@ -190,12 +190,12 @@ public abstract class * @see #opt(String, String) */ @Override - public final B opt(@Nonnull final String key, int value) { + public B opt(@Nonnull final String key, int value) { return optLong(key, value); } @Override - public final B opt(@Nonnull final String key, final long value) { + public B opt(@Nonnull final String key, final long value) { return optLong(key, value); } @@ -210,7 +210,7 @@ public abstract class * @see #opt(String, String) */ @Override - public final B opt(@Nonnull final String key, float value) { + public B opt(@Nonnull final String key, float value) { return optLong(key, (long) value); } @@ -220,7 +220,7 @@ public abstract class * @see #opt(String, String) */ @Override - public final B opt(@Nonnull final String key, double value) { + public B opt(@Nonnull final String key, double value) { return optLong(key, (long) value); } @@ -291,22 +291,22 @@ public abstract class * @see #must(String, String) */ @Override - public final B must(@Nonnull final String key, int value) { + public B must(@Nonnull final String key, int value) { return mustLong(key, value); } @Override - public final B must(@Nonnull final String key, final long value) { + public B must(@Nonnull final String key, final long value) { return mustLong(key, value); } @Override - public final B must(@Nonnull final String key, final float value) { + public B must(@Nonnull final String key, final float value) { return mustLong(key, (long) value); } @Override - public final B must(@Nonnull final String key, double value) { + public B must(@Nonnull final String key, double value) { return mustLong(key, (long) value); } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/store/TestFSBuilderSupport.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/store/TestFSBuilderSupport.java index 20172ccfe16..c34cdbe0ae5 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/store/TestFSBuilderSupport.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/store/TestFSBuilderSupport.java @@ -20,11 +20,12 @@ package org.apache.hadoop.fs.store; import java.io.IOException; +import javax.annotation.Nonnull; + import org.junit.Test; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FSBuilder; -import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.impl.AbstractFSBuilderImpl; import org.apache.hadoop.fs.impl.FSBuilderSupport; import org.apache.hadoop.test.AbstractHadoopTestBase; @@ -127,18 +128,45 @@ public class TestFSBuilderSupport extends AbstractHadoopTestBase { extends FSBuilder { } + /** + * This is a minimal builder which relies on default implementations of the interface. + * If it ever stops compiling, it means a new interface has been added which + * is not backwards compatible with external implementations, such as that + * in HBoss (see HBASE-26483). + * + */ private static final class BuilderImpl - extends AbstractFSBuilderImpl implements SimpleBuilder { + private final Configuration options = new Configuration(false); - private BuilderImpl() { - super(new Path("/")); + @Override + public SimpleBuilder opt(@Nonnull final String key, @Nonnull final String value) { + options.set(key, value); + return this; + } + + @Override + public SimpleBuilder opt(@Nonnull final String key, @Nonnull final String... values) { + options.setStrings(key, values); + return this; + } + + @Override + public SimpleBuilder must(@Nonnull final String key, @Nonnull final String value) { + return opt(key, value); + } + + @Override + public SimpleBuilder must(@Nonnull final String key, @Nonnull final String... values) { + return opt(key, values); } @Override public FSBuilderSupport build() - throws IOException { - return new FSBuilderSupport(getOptions()); + throws IllegalArgumentException, UnsupportedOperationException, IOException { + return new FSBuilderSupport(options); } } + + }