[
https://issues.apache.org/jira/browse/BEAM-308?focusedWorklogId=160660&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-160660
]
ASF GitHub Bot logged work on BEAM-308:
---------------------------------------
Author: ASF GitHub Bot
Created on: 30/Oct/18 14:36
Start Date: 30/Oct/18 14:36
Worklog Time Spent: 10m
Work Description: swegner closed pull request #6871: [BEAM-308] Print
warning about using non-public PipelineOptions interface.
URL: https://github.com/apache/beam/pull/6871
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/examples/java/src/main/java/org/apache/beam/examples/complete/StreamingWordExtract.java
b/examples/java/src/main/java/org/apache/beam/examples/complete/StreamingWordExtract.java
index 978836d3454..4cfbcd56b98 100644
---
a/examples/java/src/main/java/org/apache/beam/examples/complete/StreamingWordExtract.java
+++
b/examples/java/src/main/java/org/apache/beam/examples/complete/StreamingWordExtract.java
@@ -95,7 +95,7 @@ static TableSchema getSchema() {
*
* <p>Inherits standard configuration options.
*/
- private interface StreamingWordExtractOptions
+ public interface StreamingWordExtractOptions
extends ExampleOptions, ExampleBigQueryTableOptions, StreamingOptions {
@Description("Path of the file to read from")
@Default.String("gs://apache-beam-samples/shakespeare/kinglear.txt")
diff --git
a/examples/java/src/main/java/org/apache/beam/examples/complete/game/StatefulTeamScore.java
b/examples/java/src/main/java/org/apache/beam/examples/complete/game/StatefulTeamScore.java
index ccfe9f8e40a..15a63f70425 100644
---
a/examples/java/src/main/java/org/apache/beam/examples/complete/game/StatefulTeamScore.java
+++
b/examples/java/src/main/java/org/apache/beam/examples/complete/game/StatefulTeamScore.java
@@ -79,7 +79,7 @@
public class StatefulTeamScore extends LeaderBoard {
/** Options supported by {@link StatefulTeamScore}. */
- interface Options extends LeaderBoard.Options {
+ public interface Options extends LeaderBoard.Options {
@Description("Numeric value, multiple of which is used as threshold for
outputting team score.")
@Default.Integer(5000)
diff --git
a/examples/java/src/main/java/org/apache/beam/examples/cookbook/CombinePerKeyExamples.java
b/examples/java/src/main/java/org/apache/beam/examples/cookbook/CombinePerKeyExamples.java
index da5e90d8ee8..d06912f81d2 100644
---
a/examples/java/src/main/java/org/apache/beam/examples/cookbook/CombinePerKeyExamples.java
+++
b/examples/java/src/main/java/org/apache/beam/examples/cookbook/CombinePerKeyExamples.java
@@ -159,7 +159,7 @@ public String apply(Iterable<String> input) {
*
* <p>Inherits standard configuration options.
*/
- private interface Options extends PipelineOptions {
+ public interface Options extends PipelineOptions {
@Description("Table to read from, specified as " +
"<project_id>:<dataset_id>.<table_id>")
@Default.String(SHAKESPEARE_TABLE)
String getInput();
diff --git
a/examples/java/src/main/java/org/apache/beam/examples/cookbook/DistinctExample.java
b/examples/java/src/main/java/org/apache/beam/examples/cookbook/DistinctExample.java
index 9efa2d1d9e3..0f6a52f5b49 100644
---
a/examples/java/src/main/java/org/apache/beam/examples/cookbook/DistinctExample.java
+++
b/examples/java/src/main/java/org/apache/beam/examples/cookbook/DistinctExample.java
@@ -56,7 +56,7 @@
*
* <p>Inherits standard configuration options.
*/
- private interface Options extends PipelineOptions {
+ public interface Options extends PipelineOptions {
@Description("Path to the directory or GCS prefix containing files to read
from")
@Default.String("gs://apache-beam-samples/shakespeare/*")
String getInput();
diff --git
a/examples/java/src/main/java/org/apache/beam/examples/cookbook/FilterExamples.java
b/examples/java/src/main/java/org/apache/beam/examples/cookbook/FilterExamples.java
index 17a34b00c8d..4d2480fa4e0 100644
---
a/examples/java/src/main/java/org/apache/beam/examples/cookbook/FilterExamples.java
+++
b/examples/java/src/main/java/org/apache/beam/examples/cookbook/FilterExamples.java
@@ -198,7 +198,7 @@ public void processElement(ProcessContext c) {
*
* <p>Inherits standard configuration options.
*/
- private interface Options extends PipelineOptions {
+ public interface Options extends PipelineOptions {
@Description("Table to read from, specified as " +
"<project_id>:<dataset_id>.<table_id>")
@Default.String(WEATHER_SAMPLES_TABLE)
String getInput();
diff --git
a/examples/java/src/main/java/org/apache/beam/examples/cookbook/JoinExamples.java
b/examples/java/src/main/java/org/apache/beam/examples/cookbook/JoinExamples.java
index 3b5fd6082f9..bdbe25f40b6 100644
---
a/examples/java/src/main/java/org/apache/beam/examples/cookbook/JoinExamples.java
+++
b/examples/java/src/main/java/org/apache/beam/examples/cookbook/JoinExamples.java
@@ -157,7 +157,7 @@ public void processElement(ProcessContext c) {
*
* <p>Inherits standard configuration options.
*/
- private interface Options extends PipelineOptions {
+ public interface Options extends PipelineOptions {
@Description("Path of the file to write to")
@Validation.Required
String getOutput();
diff --git
a/examples/java/src/main/java/org/apache/beam/examples/cookbook/MaxPerKeyExamples.java
b/examples/java/src/main/java/org/apache/beam/examples/cookbook/MaxPerKeyExamples.java
index 369f796502b..63d2cde5dca 100644
---
a/examples/java/src/main/java/org/apache/beam/examples/cookbook/MaxPerKeyExamples.java
+++
b/examples/java/src/main/java/org/apache/beam/examples/cookbook/MaxPerKeyExamples.java
@@ -119,7 +119,7 @@ public void processElement(ProcessContext c) {
*
* <p>Inherits standard configuration options.
*/
- private interface Options extends PipelineOptions {
+ public interface Options extends PipelineOptions {
@Description("Table to read from, specified as " +
"<project_id>:<dataset_id>.<table_id>")
@Default.String(WEATHER_SAMPLES_TABLE)
String getInput();
diff --git
a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java
b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java
index 33157eb32f2..86e6aa187a9 100644
---
a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java
+++
b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptions.java
@@ -101,7 +101,7 @@
* PipelineOptionsFactory as your command-line interpreter, you will provide a
standardized way for
* users to interact with your application via the command-line.
*
- * <p>To define your own {@link PipelineOptions}, you create an interface
which extends {@link
+ * <p>To define your own {@link PipelineOptions}, you create a public
interface which extends {@link
* PipelineOptions} and define getter/setter pairs. These getter/setter pairs
define a collection of
* <a
href="https://docs.oracle.com/javase/tutorial/javabeans/writing/properties.html";>JavaBean
* properties</a>.
diff --git
a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
index 9d0bd431cf8..3546fcd7737 100644
---
a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
+++
b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/PipelineOptionsFactory.java
@@ -865,6 +865,19 @@ private static void
throwForTypeMismatches(List<TypeMismatch> mismatches) {
Set<Class<? extends PipelineOptions>> validatedPipelineOptionsInterfaces,
Class<? extends PipelineOptions> klass)
throws IntrospectionException {
+
+ // TODO(BEAM-308): Make this an error in users pipelines for the next
major version
+ // of Apache Beam.
+ if (!Modifier.isPublic(iface.getModifiers())) {
+ LOG.warn(
+ "Using non-public interface {} may fail during runtime. The JVM
requires that "
+ + "all non-public interfaces to be in the same package;
otherwise, it would not be "
+ + "possible for the PipelineOptions proxy class to implement all
of the interfaces, "
+ + "regardless of what package it is defined in. This will become
an error in"
+ + "a future version of Apache Beam.",
+ iface.getName());
+ }
+
// Verify that there are no methods with the same name with two different
return types.
validateReturnType(iface);
diff --git
a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsFactoryTest.java
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsFactoryTest.java
index 7cca11b960c..aa81d61a32d 100644
---
a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsFactoryTest.java
+++
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsFactoryTest.java
@@ -450,7 +450,8 @@ public void testNotAllGettersAnnotatedWithJsonIgnore()
throws Exception {
options.as(CombinedObject.class);
}
- private interface MultiGetters extends PipelineOptions {
+ /** Test interface. */
+ public interface MultiGetters extends PipelineOptions {
Object getObject();
void setObject(Object value);
@@ -465,7 +466,8 @@ public void testNotAllGettersAnnotatedWithJsonIgnore()
throws Exception {
void setConsistent(Void consistent);
}
- private interface MultipleGettersWithInconsistentJsonIgnore extends
PipelineOptions {
+ /** Test interface. */
+ public interface MultipleGettersWithInconsistentJsonIgnore extends
PipelineOptions {
@JsonIgnore
Object getObject();
@@ -559,7 +561,7 @@ public void testMultipleSettersAnnotatedWithDefault()
throws Exception {
* This class is has a conflicting field with {@link CombinedObject} that
doesn't have {@link
* Default @Default}.
*/
- private interface GetterWithDefault extends PipelineOptions {
+ public interface GetterWithDefault extends PipelineOptions {
@Default.Integer(1)
Object getObject();
@@ -570,7 +572,7 @@ public void testMultipleSettersAnnotatedWithDefault()
throws Exception {
* This class is consistent with {@link GetterWithDefault} that has the same
{@link
* Default @Default}.
*/
- private interface GetterWithConsistentDefault extends PipelineOptions {
+ public interface GetterWithConsistentDefault extends PipelineOptions {
@Default.Integer(1)
Object getObject();
@@ -581,7 +583,7 @@ public void testMultipleSettersAnnotatedWithDefault()
throws Exception {
* This class is inconsistent with {@link GetterWithDefault} that has a
different {@link
* Default @Default}.
*/
- private interface GetterWithInconsistentDefaultType extends PipelineOptions {
+ public interface GetterWithInconsistentDefaultType extends PipelineOptions {
@Default.String("abc")
Object getObject();
@@ -592,7 +594,7 @@ public void testMultipleSettersAnnotatedWithDefault()
throws Exception {
* This class is inconsistent with {@link GetterWithDefault} that has a
different {@link
* Default @Default} value.
*/
- private interface GetterWithInconsistentDefaultValue extends PipelineOptions
{
+ public interface GetterWithInconsistentDefaultValue extends PipelineOptions {
@Default.Integer(0)
Object getObject();
@@ -675,7 +677,8 @@ public void
testGettersAnnotatedWithInconsistentJsonIgnoreValue() throws Excepti
options.as(GetterWithInconsistentJsonIgnoreValue.class);
}
- private interface GettersWithMultipleDefault extends PipelineOptions {
+ /** Test interface. */
+ public interface GettersWithMultipleDefault extends PipelineOptions {
@Default.String("abc")
@Default.Integer(0)
Object getObject();
@@ -697,7 +700,8 @@ public void testGettersWithMultipleDefaults() throws
Exception {
PipelineOptionsFactory.as(GettersWithMultipleDefault.class);
}
- private interface MultiGettersWithDefault extends PipelineOptions {
+ /** Test interface. */
+ public interface MultiGettersWithDefault extends PipelineOptions {
Object getObject();
void setObject(Object value);
@@ -712,7 +716,8 @@ public void testGettersWithMultipleDefaults() throws
Exception {
void setConsistent(Void consistent);
}
- private interface MultipleGettersWithInconsistentDefault extends
PipelineOptions {
+ /** Test interface. */
+ public interface MultipleGettersWithInconsistentDefault extends
PipelineOptions {
@Default.Boolean(true)
Object getObject();
@@ -1006,7 +1011,7 @@ public ComplexType(@JsonProperty("key") String value,
@JsonProperty("key2") Stri
}
/** A test interface for verifying JSON -> complex type conversion. */
- interface ComplexTypes extends PipelineOptions {
+ public interface ComplexTypes extends PipelineOptions {
Map<String, String> getMap();
void setMap(Map<String, String> value);
@@ -1421,6 +1426,16 @@ public void
testSetASingularAttributeUsingAListIsIgnoredWithoutStrictParsing() {
expectedLogs.verifyWarn("Strict parsing is disabled, ignoring option");
}
+ private interface NonPublicPipelineOptions extends PipelineOptions {}
+
+ @Test
+ public void testNonPublicInterfaceLogsWarning() throws Exception {
+ PipelineOptionsFactory.as(NonPublicPipelineOptions.class);
+ // Make sure we print the name of the class.
+ expectedLogs.verifyWarn(NonPublicPipelineOptions.class.getName());
+ expectedLogs.verifyWarn("all non-public interfaces to be in the same
package");
+ }
+
/** A test interface containing all supported List return types. */
public interface Maps extends PipelineOptions {
Map<Integer, Integer> getMap();
@@ -1533,7 +1548,8 @@ public void
testUsingArgumentWithUnknownPropertyIsNotAllowed() {
PipelineOptionsFactory.fromArgs(args).create();
}
- interface SuggestedOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface SuggestedOptions extends PipelineOptions {
String getAbc();
void setAbc(String value);
@@ -1685,15 +1701,15 @@ public void
testSpecificHelpAsArgumentWithClassNameSuffix() {
}
/** Used for a name collision test with the other NameConflict interfaces. */
- private static class NameConflictClassA {
+ public static class NameConflictClassA {
/** Used for a name collision test with the other NameConflict interfaces.
*/
- private interface NameConflict extends PipelineOptions {}
+ public interface NameConflict extends PipelineOptions {}
}
/** Used for a name collision test with the other NameConflict interfaces. */
- private static class NameConflictClassB {
+ public static class NameConflictClassB {
/** Used for a name collision test with the other NameConflict interfaces.
*/
- private interface NameConflict extends PipelineOptions {}
+ public interface NameConflict extends PipelineOptions {}
}
@Test
@@ -1785,20 +1801,23 @@ public void testProgrammaticPrintHelpForSpecificType() {
output, containsString("The pipeline runner that will be used to
execute the pipeline."));
}
- interface PipelineOptionsInheritedInvalid
+ /** Test interface. */
+ public interface PipelineOptionsInheritedInvalid
extends Invalid1, InvalidPipelineOptions2, PipelineOptions {
String getFoo();
void setFoo(String value);
}
- interface InvalidPipelineOptions1 {
+ /** Test interface. */
+ public interface InvalidPipelineOptions1 {
String getBar();
void setBar(String value);
}
- interface Invalid1 extends InvalidPipelineOptions1 {
+ /** Test interface. */
+ public interface Invalid1 extends InvalidPipelineOptions1 {
@Override
String getBar();
@@ -1806,7 +1825,8 @@ public void testProgrammaticPrintHelpForSpecificType() {
void setBar(String value);
}
- interface InvalidPipelineOptions2 {
+ /** Test interface. */
+ public interface InvalidPipelineOptions2 {
String getBar();
void setBar(String value);
@@ -1864,7 +1884,8 @@ public PipelineResult run(Pipeline p) {
}
}
- private interface RegisteredTestOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface RegisteredTestOptions extends PipelineOptions {
Object getRegisteredExampleFooBar();
void setRegisteredExampleFooBar(Object registeredExampleFooBar);
@@ -1890,7 +1911,7 @@ public void
testRegistrationOfJacksonModulesForObjectMapper() throws Exception {
}
/** PipelineOptions used to test auto registration of Jackson modules. */
- interface JacksonIncompatibleOptions extends PipelineOptions {
+ public interface JacksonIncompatibleOptions extends PipelineOptions {
JacksonIncompatible getJacksonIncompatible();
void setJacksonIncompatible(JacksonIncompatible value);
@@ -1988,7 +2009,8 @@ public void
testDefaultMethodIgnoresDefaultImplementation() {
assertThat(optsWithDefault.getValue(), equalTo(12.25));
}
- private interface ExtendedOptionsWithDefault extends
OptionsWithDefaultMethod {}
+ /** Test interface. */
+ public interface ExtendedOptionsWithDefault extends OptionsWithDefaultMethod
{}
@Test
public void testDefaultMethodInExtendedClassIgnoresDefaultImplementation() {
@@ -2000,7 +2022,8 @@ public void
testDefaultMethodInExtendedClassIgnoresDefaultImplementation() {
assertThat(extendedOptsWithDefault.getValue(),
equalTo(Double.NEGATIVE_INFINITY));
}
- private interface OptionsWithDefaultMethod extends PipelineOptions {
+ /** Test interface. */
+ public interface OptionsWithDefaultMethod extends PipelineOptions {
default Number getValue() {
return 1024;
}
@@ -2016,7 +2039,8 @@ public void testStaticMethodsAreAllowed() {
PipelineOptionsFactory.fromArgs("--myMethod=value").as(OptionsWithStaticMethod.class)));
}
- private interface OptionsWithStaticMethod extends PipelineOptions {
+ /** Test interface. */
+ public interface OptionsWithStaticMethod extends PipelineOptions {
String getMyMethod();
void setMyMethod(String value);
diff --git
a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsReflectorTest.java
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsReflectorTest.java
index 360525ee325..0766fb2e17e 100644
---
a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsReflectorTest.java
+++
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsReflectorTest.java
@@ -51,7 +51,8 @@ public void testGetOptionSpecs() throws NoSuchMethodException
{
SimpleOptions.class, "foo",
SimpleOptions.class.getDeclaredMethod("getFoo"))));
}
- interface SimpleOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface SimpleOptions extends PipelineOptions {
String getFoo();
void setFoo(String value);
@@ -65,7 +66,8 @@ public void testFiltersNonGetterMethods() {
assertThat(properties, not(hasItem(hasName(isOneOf("misspelled",
"hasParameter", "prefix")))));
}
- interface OnlyTwoValidGetters extends PipelineOptions {
+ /** Test interface. */
+ public interface OnlyTwoValidGetters extends PipelineOptions {
String getFoo();
void setFoo(String value);
@@ -97,7 +99,8 @@ public void testBaseClassOptions() {
assertThat(props, hasItem(allOf(hasName("bar"),
hasClass(ExtendsSimpleOptions.class))));
}
- interface ExtendsSimpleOptions extends SimpleOptions {
+ /** Test interface. */
+ public interface ExtendsSimpleOptions extends SimpleOptions {
@Override
String getFoo();
@@ -117,13 +120,15 @@ public void testExcludesNonPipelineOptionsMethods() {
assertThat(properties, not(hasItem(hasName("foo"))));
}
- interface NoExtendsClause {
+ /** Test interface. */
+ public interface NoExtendsClause {
String getFoo();
void setFoo(String value);
}
- interface ExtendsNonPipelineOptions extends NoExtendsClause, PipelineOptions
{}
+ /** Test interface. */
+ public interface ExtendsNonPipelineOptions extends NoExtendsClause,
PipelineOptions {}
@Test
public void testExcludesHiddenInterfaces() {
@@ -133,8 +138,9 @@ public void testExcludesHiddenInterfaces() {
assertThat(properties, not(hasItem(hasName("foo"))));
}
+ /** Test interface. */
@Hidden
- interface HiddenOptions extends PipelineOptions {
+ public interface HiddenOptions extends PipelineOptions {
String getFoo();
void setFoo(String value);
@@ -149,7 +155,8 @@ public void testShouldSerialize() {
assertThat(properties, hasItem(allOf(hasName("ignored"),
not(shouldSerialize()))));
}
- interface JsonIgnoreOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface JsonIgnoreOptions extends PipelineOptions {
String getNotIgnored();
void setNotIgnored(String value);
@@ -172,19 +179,22 @@ public void testMultipleInputInterfaces() {
assertThat(props, hasItem(allOf(hasName("extendOption2"),
hasClass(ExtendOptions2.class))));
}
- interface BaseOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface BaseOptions extends PipelineOptions {
String getBaseOption();
void setBaseOption(String value);
}
- interface ExtendOptions1 extends BaseOptions {
+ /** Test interface. */
+ public interface ExtendOptions1 extends BaseOptions {
String getExtendOption1();
void setExtendOption1(String value);
}
- interface ExtendOptions2 extends BaseOptions {
+ /** Test interface. */
+ public interface ExtendOptions2 extends BaseOptions {
String getExtendOption2();
void setExtendOption2(String value);
diff --git
a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsTest.java
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsTest.java
index 167f689f80e..d386dc2497a 100644
---
a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsTest.java
+++
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsTest.java
@@ -42,7 +42,7 @@
@Rule public ExpectedException expectedException = ExpectedException.none();
/** Interfaces used for testing that {@link PipelineOptions#as(Class)}
functions. */
- private interface DerivedTestOptions extends BaseTestOptions {
+ public interface DerivedTestOptions extends BaseTestOptions {
int getDerivedValue();
void setDerivedValue(int derivedValue);
@@ -55,7 +55,8 @@
void setIgnoredValue(Set<String> ignoredValue);
}
- private interface ConflictedTestOptions extends BaseTestOptions {
+ /** Test interface. */
+ public interface ConflictedTestOptions extends BaseTestOptions {
String getDerivedValue();
void setDerivedValue(String derivedValue);
@@ -68,7 +69,8 @@
void setIgnoredValue(Set<String> ignoredValue);
}
- private interface BaseTestOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface BaseTestOptions extends PipelineOptions {
List<Boolean> getBaseValue();
void setBaseValue(List<Boolean> baseValue);
@@ -85,7 +87,8 @@ public void testDynamicAs() {
assertNotNull(options);
}
- private interface ValueProviderOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface ValueProviderOptions extends PipelineOptions {
ValueProvider<Boolean> getBool();
void setBool(ValueProvider<Boolean> value);
diff --git
a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsValidatorTest.java
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsValidatorTest.java
index 6c2b335e1c2..a98d0276c49 100644
---
a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsValidatorTest.java
+++
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/PipelineOptionsValidatorTest.java
@@ -218,7 +218,8 @@ public void
testWhenOneOfMultipleRequiredGroupsIsSetIsValid() {
PipelineOptionsValidator.validate(MultiGroupRequired.class,
multiGroupRequired);
}
- private interface LeftOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface LeftOptions extends PipelineOptions {
@Validation.Required(groups = {"left"})
String getFoo();
@@ -235,7 +236,8 @@ public void
testWhenOneOfMultipleRequiredGroupsIsSetIsValid() {
void setBoth(String both);
}
- private interface RightOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface RightOptions extends PipelineOptions {
@Validation.Required(groups = {"right"})
String getFoo();
@@ -252,7 +254,8 @@ public void
testWhenOneOfMultipleRequiredGroupsIsSetIsValid() {
void setBoth(String both);
}
- private interface JoinedOptions extends LeftOptions, RightOptions {}
+ /** Test interface. */
+ public interface JoinedOptions extends LeftOptions, RightOptions {}
@Test
public void
testWhenOptionIsDefinedInMultipleSuperInterfacesAndIsNotPresentFailsRequirement()
{
@@ -312,7 +315,8 @@ public void
testWhenOptionIsDefinedOnMultipleInterfacesOnlyListedOnceWhenNotPres
PipelineOptionsValidator.validate(JoinedOptions.class, options);
}
- private interface SuperOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface SuperOptions extends PipelineOptions {
@Validation.Required(groups = {"super"})
String getFoo();
@@ -329,7 +333,8 @@ public void
testWhenOptionIsDefinedOnMultipleInterfacesOnlyListedOnceWhenNotPres
void setSuperclassObj(String sup);
}
- private interface SubOptions extends SuperOptions {
+ /** Test interface. */
+ public interface SubOptions extends SuperOptions {
@Override
@Validation.Required(groups = {"sub"})
String getFoo();
diff --git
a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ProxyInvocationHandlerTest.java
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ProxyInvocationHandlerTest.java
index 1fbb786cd7a..049852d29ac 100644
---
a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ProxyInvocationHandlerTest.java
+++
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ProxyInvocationHandlerTest.java
@@ -542,7 +542,7 @@ public void testJsonConversionForDefault() throws Exception
{
}
/** Test interface for JSON conversion of simple types. */
- private interface SimpleTypes extends PipelineOptions {
+ public interface SimpleTypes extends PipelineOptions {
int getInteger();
void setInteger(int value);
@@ -603,7 +603,7 @@ public void
testJsonConversionForOverriddenSerializedValues() throws Exception {
}
/** Test interface for JSON conversion of container types. */
- private interface ContainerTypes extends PipelineOptions {
+ public interface ContainerTypes extends PipelineOptions {
List<String> getList();
void setList(List<String> values);
@@ -633,7 +633,7 @@ public void testJsonConversionForContainerTypes() throws
Exception {
}
/** Test interface for conversion of inner types. */
- private static class InnerType {
+ public static class InnerType {
public double doubleField;
static InnerType of(double value) {
@@ -656,7 +656,7 @@ public boolean equals(Object obj) {
}
/** Test interface for conversion of generics and inner types. */
- private static class ComplexType {
+ public static class ComplexType {
public String stringField;
public Integer intField;
public List<InnerType> genericType;
@@ -678,7 +678,8 @@ public boolean equals(Object obj) {
}
}
- private interface ComplexTypes extends PipelineOptions {
+ /** Test interface. */
+ public interface ComplexTypes extends PipelineOptions {
ComplexType getComplexType();
void setComplexType(ComplexType value);
@@ -699,7 +700,7 @@ public void testJsonConversionForComplexType() throws
Exception {
}
/** Test interface for testing ignored properties during serialization. */
- private interface IgnoredProperty extends PipelineOptions {
+ public interface IgnoredProperty extends PipelineOptions {
@JsonIgnore
String getValue();
@@ -729,7 +730,7 @@ public String getValue() {
}
/** Test interface containing a class that is not serializable by Jackson. */
- private interface NotSerializableProperty extends PipelineOptions {
+ public interface NotSerializableProperty extends PipelineOptions {
NotSerializable getValue();
void setValue(NotSerializable value);
@@ -749,7 +750,7 @@ public void testJsonConversionOfNotSerializableProperty()
throws Exception {
* Test interface that has {@link JsonIgnore @JsonIgnore} on a property that
Jackson can't
* serialize.
*/
- private interface IgnoredNotSerializableProperty extends PipelineOptions {
+ public interface IgnoredNotSerializableProperty extends PipelineOptions {
@JsonIgnore
NotSerializable getValue();
@@ -785,7 +786,7 @@ public String getValue() {
* Test interface containing a property that is serializable by Jackson only
with the additional
* metadata.
*/
- private interface SerializableWithMetadataProperty extends PipelineOptions {
+ public interface SerializableWithMetadataProperty extends PipelineOptions {
SerializableWithMetadata getValue();
void setValue(SerializableWithMetadata value);
@@ -842,7 +843,8 @@ public String toString() {
assertThat(displayData, hasDisplayItem("object", "foobar"));
}
- interface TypedOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface TypedOptions extends PipelineOptions {
int getInteger();
void setInteger(int value);
@@ -906,13 +908,15 @@ public void testDisplayDataInheritanceNamespace() {
allOf(hasKey("foo"), hasValue("bar"),
hasNamespace(ExtendsBaseOptions.class))));
}
- interface BaseOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface BaseOptions extends PipelineOptions {
String getFoo();
void setFoo(String value);
}
- interface ExtendsBaseOptions extends BaseOptions {
+ /** Test interface. */
+ public interface ExtendsBaseOptions extends BaseOptions {
@Override
String getFoo();
@@ -942,13 +946,15 @@ public void
testDisplayDataIncludedForDisjointInterfaceHierarchies() {
assertThat(data, hasDisplayItem(allOf(hasKey("bar"),
hasNamespace(BarOptions.class))));
}
- interface FooOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface FooOptions extends PipelineOptions {
String getFoo();
void setFoo(String value);
}
- interface BarOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface BarOptions extends PipelineOptions {
String getBar();
void setBar(String value);
@@ -962,7 +968,8 @@ public void testDisplayDataExcludesDefaultValues() {
assertThat(data, not(hasDisplayItem("foo")));
}
- interface HasDefaults extends PipelineOptions {
+ /** Test interface. */
+ public interface HasDefaults extends PipelineOptions {
@Default.String("bar")
String getFoo();
@@ -1016,7 +1023,8 @@ public void testDisplayDataArrayValue() throws Exception {
assertThat(deserializedData, hasDisplayItem("deepPrimitiveArray", "[[1,
2], [3]]"));
}
- private interface ArrayOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface ArrayOptions extends PipelineOptions {
String[][] getDeepArray();
void setDeepArray(String[][] value);
@@ -1084,7 +1092,8 @@ public void
testDisplayDataMissingPipelineOptionsRegistration() throws Exception
assertThat(displayData, hasDisplayItem("classOption", expectedJsonValue));
}
- interface HasClassOptions extends PipelineOptions {
+ /** Test interface. */
+ public interface HasClassOptions extends PipelineOptions {
Class<?> getClassOption();
void setClassOption(Class<?> value);
diff --git
a/sdks/java/core/src/test/java/org/apache/beam/sdk/util/common/ReflectHelpersTest.java
b/sdks/java/core/src/test/java/org/apache/beam/sdk/util/common/ReflectHelpersTest.java
index f7fab5f15c5..32568d7b61c 100644
---
a/sdks/java/core/src/test/java/org/apache/beam/sdk/util/common/ReflectHelpersTest.java
+++
b/sdks/java/core/src/test/java/org/apache/beam/sdk/util/common/ReflectHelpersTest.java
@@ -127,7 +127,8 @@ public void testTypeFormatterWithGenerics() throws
Exception {
new TypeDescriptor<Map<? super InputT, ? extends OutputT>>()
{}.getType()));
}
- private interface Options extends PipelineOptions {
+ /** Test interface. */
+ public interface Options extends PipelineOptions {
@Default.String("package.OuterClass$InnerClass#method()")
String getString();
diff --git
a/sdks/java/io/hadoop-input-format/src/test/java/org/apache/beam/sdk/io/hadoop/inputformat/HIFITestOptions.java
b/sdks/java/io/hadoop-input-format/src/test/java/org/apache/beam/sdk/io/hadoop/inputformat/HIFITestOptions.java
index 34c62f0c887..7c571b10df6 100644
---
a/sdks/java/io/hadoop-input-format/src/test/java/org/apache/beam/sdk/io/hadoop/inputformat/HIFITestOptions.java
+++
b/sdks/java/io/hadoop-input-format/src/test/java/org/apache/beam/sdk/io/hadoop/inputformat/HIFITestOptions.java
@@ -22,7 +22,7 @@
import org.apache.beam.sdk.testing.TestPipelineOptions;
/** Properties needed when using HadoopInputFormatIO with the Beam SDK. */
-interface HIFITestOptions extends TestPipelineOptions {
+public interface HIFITestOptions extends TestPipelineOptions {
//Cassandra test options
@Description("Cassandra Server IP")
diff --git
a/sdks/java/io/hcatalog/src/test/java/org/apache/beam/sdk/io/hcatalog/HCatalogIOIT.java
b/sdks/java/io/hcatalog/src/test/java/org/apache/beam/sdk/io/hcatalog/HCatalogIOIT.java
index 1ceba98d4d2..c0dc353b5f6 100644
---
a/sdks/java/io/hcatalog/src/test/java/org/apache/beam/sdk/io/hcatalog/HCatalogIOIT.java
+++
b/sdks/java/io/hcatalog/src/test/java/org/apache/beam/sdk/io/hcatalog/HCatalogIOIT.java
@@ -64,7 +64,8 @@
@RunWith(JUnit4.class)
public class HCatalogIOIT {
- private interface HCatalogPipelineOptions extends IOTestPipelineOptions {
+ /** PipelineOptions for testing {@link
org.apache.beam.sdk.io.hcatalog.HCatalogIO}. */
+ public interface HCatalogPipelineOptions extends IOTestPipelineOptions {
@Description("HCatalog metastore host (hostname/ip address)")
@Default.String("hcatalog-metastore")
String getHCatalogMetastoreHostName();
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 160660)
Time Spent: 1h (was: 50m)
> PipelineOptions throws when using package-private PipelineOptions interfaces
> from multiple packages
> ---------------------------------------------------------------------------------------------------
>
> Key: BEAM-308
> URL: https://issues.apache.org/jira/browse/BEAM-308
> Project: Beam
> Issue Type: Bug
> Components: sdk-java-core
> Reporter: Scott Wegner
> Priority: Minor
> Labels: backwards-incompatible
> Time Spent: 1h
> Remaining Estimate: 0h
>
> If a PipelineOptions instance is used as multiple PipelineOptions
> package-private interfaces from different packages, {{PipelineOptions.as}}
> will throw an exception:
> {quote}
> java.lang.IllegalArgumentException: non-public interfaces from different
> packages
> at java.lang.reflect.Proxy$ProxyClassFactory.apply(Proxy.java:652)
> at java.lang.reflect.Proxy$ProxyClassFactory.apply(Proxy.java:592)
> at java.lang.reflect.WeakCache$Factory.get(WeakCache.java:244)
> at java.lang.reflect.WeakCache.get(WeakCache.java:141)
> at java.lang.reflect.Proxy.getProxyClass0(Proxy.java:455)
> at java.lang.reflect.Proxy.getProxyClass(Proxy.java:405)
> at
> org.apache.beam.sdk.options.PipelineOptionsFactory.validateWellFormed(PipelineOptionsFactory.java:620)
> at
> org.apache.beam.sdk.options.ProxyInvocationHandler.as(ProxyInvocationHandler.java:209)
> at
> org.apache.beam.sdk.options.ProxyInvocationHandler.invoke(ProxyInvocationHandler.java:135)
> at com.sun.proxy.$Proxy6.as(Unknown Source)
> {quote}
> This fails because ProxyInvocationHandler attempts to create a Java Proxy
> object implementing the full set of interfaces, which has the
> [restriction|https://docs.oracle.com/javase/7/docs/api/java/lang/reflect/Proxy.html#getProxyClass(java.lang.ClassLoader,%20java.lang.Class...)]:
>
> bq. All non-public interfaces must be in the same package; otherwise, it
> would not be possible for the proxy class to implement all of the interfaces,
> regardless of what package it is defined in.
> This can be triggered in a couple edge-case scenarios:
> # {{PipelineOptions.as}} is called on the same PipelineOptions instance with
> multiple package-private interfaces.
> # {{PipelineOptionsFactory.register}} is called with a package-private
> interface, and then {{PipelineOptions.as}} is called with a different
> package-private instance.
> We hit the second scenario in [DataflowSDK unit
> tests|https://github.com/GoogleCloudPlatform/DataflowJavaSDK/pull/286#issuecomment-221438063].
> It's hard to trigger, but possible.
> ----
> I propose we make the behavior for package-private options explicit:
> # Give a better exception message if we hit this issue in
> {{PipelineOptions.as}} listing the non-public interfaces and what packages
> they're in.
> # Explicitly reject non-public interfaces from
> {{PipelineOptionsFactory.register}}, since this state is global and is easier
> to cause issues.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
