[ 
https://issues.apache.org/jira/browse/BEAM-3842?focusedWorklogId=79987&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-79987
 ]

ASF GitHub Bot logged work on BEAM-3842:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 13/Mar/18 17:53
            Start Date: 13/Mar/18 17:53
    Worklog Time Spent: 10m 
      Work Description: lukecwik closed pull request #4855: [BEAM-3842, 
BEAM-3843] Enable static methods and migrate to hasExperiments on 
ExperimentalOptions
URL: https://github.com/apache/beam/pull/4855
 
 
   

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/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowPipelineTranslator.java
 
b/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowPipelineTranslator.java
index a0fd9943768..14e55c35b33 100644
--- 
a/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowPipelineTranslator.java
+++ 
b/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowPipelineTranslator.java
@@ -21,7 +21,6 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Strings.isNullOrEmpty;
-import static org.apache.beam.runners.dataflow.DataflowRunner.hasExperiment;
 import static org.apache.beam.runners.dataflow.util.Structs.addBoolean;
 import static org.apache.beam.runners.dataflow.util.Structs.addDictionary;
 import static org.apache.beam.runners.dataflow.util.Structs.addList;
@@ -29,6 +28,7 @@
 import static org.apache.beam.runners.dataflow.util.Structs.addObject;
 import static org.apache.beam.runners.dataflow.util.Structs.addString;
 import static org.apache.beam.runners.dataflow.util.Structs.getString;
+import static org.apache.beam.sdk.options.ExperimentalOptions.hasExperiment;
 import static org.apache.beam.sdk.util.SerializableUtils.serializeToByteArray;
 import static org.apache.beam.sdk.util.StringUtils.byteArrayToJsonString;
 
diff --git 
a/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/TestDataflowRunner.java
 
b/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/TestDataflowRunner.java
index c3108b69a87..e163fe8d674 100644
--- 
a/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/TestDataflowRunner.java
+++ 
b/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/TestDataflowRunner.java
@@ -17,6 +17,7 @@
  */
 package org.apache.beam.runners.dataflow;
 
+import static org.apache.beam.sdk.options.ExperimentalOptions.hasExperiment;
 import static org.hamcrest.MatcherAssert.assertThat;
 
 import com.google.api.services.dataflow.model.JobMessage;
@@ -209,7 +210,7 @@ private static String errorMessage(
 
   @VisibleForTesting
   void updatePAssertCount(Pipeline pipeline) {
-    if (DataflowRunner.hasExperiment(options, "beam_fn_api")) {
+    if (hasExperiment(options, "beam_fn_api")) {
       // TODO[BEAM-1866]: FnAPI does not support metrics, so expect 0 
assertions.
       expectedNumberOfAssertions = 0;
     } else {
diff --git 
a/runners/google-cloud-dataflow-java/src/test/java/org/apache/beam/runners/dataflow/DataflowRunnerTest.java
 
b/runners/google-cloud-dataflow-java/src/test/java/org/apache/beam/runners/dataflow/DataflowRunnerTest.java
index 4520c057a3c..a8b128dc305 100644
--- 
a/runners/google-cloud-dataflow-java/src/test/java/org/apache/beam/runners/dataflow/DataflowRunnerTest.java
+++ 
b/runners/google-cloud-dataflow-java/src/test/java/org/apache/beam/runners/dataflow/DataflowRunnerTest.java
@@ -28,7 +28,6 @@
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.startsWith;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
@@ -1290,22 +1289,6 @@ public void testTemplateRunnerLoggedErrorForFile() 
throws Exception {
     p.run();
   }
 
-  @Test
-  public void testHasExperiment() {
-    DataflowPipelineDebugOptions options =
-        PipelineOptionsFactory.as(DataflowPipelineDebugOptions.class);
-
-    options.setExperiments(null);
-    assertFalse(DataflowRunner.hasExperiment(options, "foo"));
-
-    options.setExperiments(ImmutableList.of("foo", "bar"));
-    assertTrue(DataflowRunner.hasExperiment(options, "foo"));
-    assertTrue(DataflowRunner.hasExperiment(options, "bar"));
-    assertFalse(DataflowRunner.hasExperiment(options, "baz"));
-    assertFalse(DataflowRunner.hasExperiment(options, "ba"));
-    assertFalse(DataflowRunner.hasExperiment(options, "BAR"));
-  }
-
   @Test
   public void testWorkerHarnessContainerImage() {
     DataflowPipelineOptions options = 
PipelineOptionsFactory.as(DataflowPipelineOptions.class);
diff --git 
a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ExperimentalOptions.java
 
b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ExperimentalOptions.java
index cb5c41c1182..05de22cc99b 100644
--- 
a/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ExperimentalOptions.java
+++ 
b/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ExperimentalOptions.java
@@ -35,4 +35,18 @@
   @Nullable
   List<String> getExperiments();
   void setExperiments(@Nullable List<String> value);
+
+  /**
+   * Returns true iff the provided pipeline options has the specified 
experiment
+   * enabled.
+   */
+  static boolean hasExperiment(PipelineOptions options, String experiment) {
+    if (options == null) {
+      return false;
+    }
+
+    List<String> experiments = 
options.as(ExperimentalOptions.class).getExperiments();
+    return experiments != null
+        && experiments.contains(experiment);
+  }
 }
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 e20addd36db..7630fb3e167 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
@@ -477,6 +477,8 @@ public Registration(Class<T> proxyClass, 
List<PropertyDescriptor> beanInfo) {
 
   /** A predicate that checks if a method is synthetic via {@link 
Method#isSynthetic()}. */
   private static final Predicate<Method> NOT_SYNTHETIC_PREDICATE = input -> 
!input.isSynthetic();
+  private static final Predicate<Method> NOT_STATIC_PREDICATE =
+      input -> !Modifier.isStatic(input.getModifiers());
 
   /** Ensure all classloader or volatile data are contained in a single 
reference. */
   static final AtomicReference<Cache> CACHE = new AtomicReference<>();
@@ -865,6 +867,7 @@ private static void 
throwForTypeMismatches(List<TypeMismatch> mismatches) {
             validatedPipelineOptionsInterfaces))
         .append(ReflectHelpers.getClosureOfMethodsOnInterface(iface))
         .filter(NOT_SYNTHETIC_PREDICATE)
+        .filter(NOT_STATIC_PREDICATE)
         .toSortedSet(MethodComparator.INSTANCE);
 
     List<PropertyDescriptor> descriptors = 
getPropertyDescriptors(allInterfaceMethods, iface);
@@ -1135,7 +1138,9 @@ private static void 
validateMethodsAreEitherBeanMethodOrKnownMethod(
         Sets.filter(
             Sets.difference(Sets.newHashSet(iface.getMethods()), knownMethods),
             Predicates.and(
-                NOT_SYNTHETIC_PREDICATE, input -> 
!knownMethodsNames.contains(input.getName()))));
+                NOT_SYNTHETIC_PREDICATE,
+                input -> !knownMethodsNames.contains(input.getName()),
+                NOT_STATIC_PREDICATE)));
     checkArgument(unknownMethods.isEmpty(),
         "Methods %s on [%s] do not conform to being bean properties.",
         
FluentIterable.from(unknownMethods).transform(ReflectHelpers.METHOD_FORMATTER),
diff --git 
a/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ExperimentalOptionsTest.java
 
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ExperimentalOptionsTest.java
new file mode 100644
index 00000000000..c60007aaf02
--- /dev/null
+++ 
b/sdks/java/core/src/test/java/org/apache/beam/sdk/options/ExperimentalOptionsTest.java
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.options;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link ExperimentalOptions}. */
+@RunWith(JUnit4.class)
+public class ExperimentalOptionsTest {
+  @Test
+  public void testExperimentIsSet() {
+    ExperimentalOptions options =
+        
PipelineOptionsFactory.fromArgs("--experiments=experimentA,experimentB")
+            .as(ExperimentalOptions.class);
+    assertTrue(ExperimentalOptions.hasExperiment(options, "experimentA"));
+    assertTrue(ExperimentalOptions.hasExperiment(options, "experimentB"));
+    assertFalse(ExperimentalOptions.hasExperiment(options, "experimentC"));
+  }
+}
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 e4c4102d6b6..be92f2ac4d0 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
@@ -1859,5 +1859,20 @@ default Number getValue() {
     void setValue(Number value);
   }
 
+  @Test
+  public void testStaticMethodsAreAllowed() {
+    assertEquals("value",
+        OptionsWithStaticMethod.myStaticMethod(
+            PipelineOptionsFactory.fromArgs("--myMethod=value")
+                .as(OptionsWithStaticMethod.class)));
+  }
+
+  private interface OptionsWithStaticMethod extends PipelineOptions {
+    String getMyMethod();
+    void setMyMethod(String value);
 
+    static String myStaticMethod(OptionsWithStaticMethod o) {
+      return o.getMyMethod();
+    }
+  }
 }


 

----------------------------------------------------------------
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:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 79987)
    Time Spent: 20m  (was: 10m)

> Allow static methods to be defined within PipelineOptions interfaces.
> ---------------------------------------------------------------------
>
>                 Key: BEAM-3842
>                 URL: https://issues.apache.org/jira/browse/BEAM-3842
>             Project: Beam
>          Issue Type: Improvement
>          Components: sdk-java-core
>            Reporter: Luke Cwik
>            Assignee: Luke Cwik
>            Priority: Minor
>          Time Spent: 20m
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to