Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BEAM-7868] optionally skip hidden pipeline options #9211

Merged
merged 1 commit into from Aug 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -592,7 +592,7 @@ public static void printHelp(PrintStream out, Class<? extends PipelineOptions> i
checkNotNull(iface);
CACHE.get().validateWellFormed(iface);

Set<PipelineOptionSpec> properties = PipelineOptionsReflector.getOptionSpecs(iface);
Set<PipelineOptionSpec> properties = PipelineOptionsReflector.getOptionSpecs(iface, true);

RowSortedTable<Class<?>, String, Method> ifacePropGetterTable =
TreeBasedTable.create(ClassNameComparator.INSTANCE, Ordering.natural());
Expand Down Expand Up @@ -661,7 +661,7 @@ public static List<PipelineOptionDescriptor> describe(
for (Class<? extends PipelineOptions> iface : ifaces) {
CACHE.get().validateWellFormed(iface);

Set<PipelineOptionSpec> properties = PipelineOptionsReflector.getOptionSpecs(iface);
Set<PipelineOptionSpec> properties = PipelineOptionsReflector.getOptionSpecs(iface, false);

RowSortedTable<Class<?>, String, Method> ifacePropGetterTable =
TreeBasedTable.create(ClassNameComparator.INSTANCE, Ordering.natural());
Expand Down
Expand Up @@ -31,12 +31,13 @@ class PipelineOptionsReflector {
private PipelineOptionsReflector() {}

/**
* Retrieve metadata for the full set of pipeline options visible within the type hierarchy of a
* single {@link PipelineOptions} interface.
* Retrieve metadata for the full set of pipeline options within the type hierarchy of a single
* {@link PipelineOptions} interface with optional filtering {@link Hidden} options.
*
* @see PipelineOptionsReflector#getOptionSpecs(Iterable)
*/
static Set<PipelineOptionSpec> getOptionSpecs(Class<? extends PipelineOptions> optionsInterface) {
static Set<PipelineOptionSpec> getOptionSpecs(
Class<? extends PipelineOptions> optionsInterface, boolean skipHidden) {
Iterable<Method> methods = ReflectHelpers.getClosureOfMethodsOnInterface(optionsInterface);
Multimap<String, Method> propsToGetters = getPropertyNamesToGetters(methods);

Expand All @@ -53,7 +54,7 @@ static Set<PipelineOptionSpec> getOptionSpecs(Class<? extends PipelineOptions> o
continue;
}

if (declaringClass.isAnnotationPresent(Hidden.class)) {
if (skipHidden && declaringClass.isAnnotationPresent(Hidden.class)) {
continue;
}

Expand All @@ -77,7 +78,7 @@ static Set<PipelineOptionSpec> getOptionSpecs(
Iterable<Class<? extends PipelineOptions>> optionsInterfaces) {
ImmutableSet.Builder<PipelineOptionSpec> setBuilder = ImmutableSet.builder();
for (Class<? extends PipelineOptions> optionsInterface : optionsInterfaces) {
setBuilder.addAll(getOptionSpecs(optionsInterface));
setBuilder.addAll(getOptionSpecs(optionsInterface, true));
}

return setBuilder.build();
Expand Down
Expand Up @@ -41,7 +41,7 @@ public class PipelineOptionsReflectorTest {
@Test
public void testGetOptionSpecs() throws NoSuchMethodException {
Set<PipelineOptionSpec> properties =
PipelineOptionsReflector.getOptionSpecs(SimpleOptions.class);
PipelineOptionsReflector.getOptionSpecs(SimpleOptions.class, true);

assertThat(
properties,
Expand All @@ -60,7 +60,7 @@ public interface SimpleOptions extends PipelineOptions {
@Test
public void testFiltersNonGetterMethods() {
Set<PipelineOptionSpec> properties =
PipelineOptionsReflector.getOptionSpecs(OnlyTwoValidGetters.class);
PipelineOptionsReflector.getOptionSpecs(OnlyTwoValidGetters.class, true);

assertThat(properties, not(hasItem(hasName(isOneOf("misspelled", "hasParameter", "prefix")))));
}
Expand Down Expand Up @@ -91,7 +91,7 @@ public interface OnlyTwoValidGetters extends PipelineOptions {
@Test
public void testBaseClassOptions() {
Set<PipelineOptionSpec> props =
PipelineOptionsReflector.getOptionSpecs(ExtendsSimpleOptions.class);
PipelineOptionsReflector.getOptionSpecs(ExtendsSimpleOptions.class, true);

assertThat(props, hasItem(allOf(hasName("foo"), hasClass(SimpleOptions.class))));
assertThat(props, hasItem(allOf(hasName("foo"), hasClass(ExtendsSimpleOptions.class))));
Expand All @@ -114,7 +114,7 @@ public interface ExtendsSimpleOptions extends SimpleOptions {
@Test
public void testExcludesNonPipelineOptionsMethods() {
Set<PipelineOptionSpec> properties =
PipelineOptionsReflector.getOptionSpecs(ExtendsNonPipelineOptions.class);
PipelineOptionsReflector.getOptionSpecs(ExtendsNonPipelineOptions.class, true);

assertThat(properties, not(hasItem(hasName("foo"))));
}
Expand All @@ -132,11 +132,19 @@ public interface ExtendsNonPipelineOptions extends NoExtendsClause, PipelineOpti
@Test
public void testExcludesHiddenInterfaces() {
Set<PipelineOptionSpec> properties =
PipelineOptionsReflector.getOptionSpecs(HiddenOptions.class);
PipelineOptionsReflector.getOptionSpecs(HiddenOptions.class, true);

assertThat(properties, not(hasItem(hasName("foo"))));
}

@Test
public void testIncludesHiddenInterfaces() {
Set<PipelineOptionSpec> properties =
PipelineOptionsReflector.getOptionSpecs(HiddenOptions.class, false);

assertThat(properties, hasItem(hasName("foo")));
}

/** Test interface. */
@Hidden
public interface HiddenOptions extends PipelineOptions {
Expand All @@ -148,7 +156,7 @@ public interface HiddenOptions extends PipelineOptions {
@Test
public void testShouldSerialize() {
Set<PipelineOptionSpec> properties =
PipelineOptionsReflector.getOptionSpecs(JsonIgnoreOptions.class);
PipelineOptionsReflector.getOptionSpecs(JsonIgnoreOptions.class, true);

assertThat(properties, hasItem(allOf(hasName("notIgnored"), shouldSerialize())));
assertThat(properties, hasItem(allOf(hasName("ignored"), not(shouldSerialize()))));
Expand Down