Clear bucket validation error message after validation success / remove unnecessary code (#2120)

* Clear error message / code cleanup

Author: chanseokoh

plugins/com.google.cloud.tools.eclipse.dataflow.ui.test/src/com/google/cloud/tools/eclipse/dataflow/ui/util/SelectFirstMatchingPrefixListenerTest.java

@@ -23,7 +23,6 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-import com.google.cloud.tools.eclipse.dataflow.ui.util.SelectFirstMatchingPrefixListener.OnCompleteListener;
 import com.google.common.collect.ImmutableSortedSet;
 import java.util.SortedSet;
 import org.eclipse.swt.events.ModifyEvent;
@@ -48,16 +47,13 @@ public class SelectFirstMatchingPrefixListenerTest {
   private SortedSet<String> elements =
       ImmutableSortedSet.of("collidingText1", "collidingText222", "exampleText");
   private SelectFirstMatchingPrefixListener listener;
-  @Mock
-  private OnCompleteListener onCompleteListener;
 
   @Before
   public void setup() {
     MockitoAnnotations.initMocks(this);
 
     listener = new SelectFirstMatchingPrefixListener(combo);
     listener.setContents(elements);
-    listener.addOnCompleteListener(onCompleteListener);
   }
 
   @Test

plugins/com.google.cloud.tools.eclipse.dataflow.ui/src/com/google/cloud/tools/eclipse/dataflow/ui/preferences/RunOptionsDefaultsComponent.java

@@ -31,9 +31,12 @@
 import com.google.cloud.tools.eclipse.dataflow.ui.util.ButtonFactory;
 import com.google.cloud.tools.eclipse.dataflow.ui.util.DisplayExecutor;
 import com.google.cloud.tools.eclipse.dataflow.ui.util.SelectFirstMatchingPrefixListener;
-import com.google.cloud.tools.eclipse.dataflow.ui.util.SelectFirstMatchingPrefixListener.OnCompleteListener;
 import com.google.cloud.tools.eclipse.ui.util.databinding.BucketNameValidator;
 import com.google.common.base.Strings;
+import java.util.Locale;
+import java.util.SortedSet;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.jface.wizard.WizardPage;
@@ -53,10 +56,6 @@
 import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Text;
 import org.eclipse.ui.statushandlers.StatusManager;
-import java.util.Locale;
-import java.util.SortedSet;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
 
 /**
  * Collects default run options for Dataflow Pipelines and provides means to create and modify them.
@@ -134,16 +133,16 @@ public RunOptionsDefaultsComponent(
 
     completionListener = new SelectFirstMatchingPrefixListener(stagingLocationInput);
     stagingLocationInput.addModifyListener(completionListener);
-    completionListener.addOnCompleteListener(new OnCompleteListener() {
+    stagingLocationInput.addModifyListener(new ModifyListener() {
       @Override
-      public void onComplete(String contents) {
-        verifyStagingLocation(contents);
+      public void modifyText(ModifyEvent event) {
+        verifyStagingLocation(stagingLocationInput.getText());
       }
     });
     createButton.addSelectionListener(new CreateStagingLocationListener());
 
     stagingLocationInput.addModifyListener(new EnableCreateButton());
-    
+
     updateStagingLocations(project);
     messageTarget.setInfo("Set Pipeline Run Option Defaults");
   }
@@ -219,25 +218,31 @@ private void updateStagingLocations(String project) {
    * Ensure the staging location specified in the input combo is valid.
    */
   private void verifyStagingLocation(final String stagingLocation) {
+    setPageComplete(false);
+    messageTarget.clear();
+
     if (verifyJob != null) {
       // Cancel any existing verifyJob
+      // FIXME: this has no effect, as "VerifyStagingLocationJob" doesn't honor cancellation.
       verifyJob.cancel();
     }
-    if (client == null) {
-      // We can't verify the staging locations because we don't have a GCS client
-      return;
-    }
-    if (stagingLocation.isEmpty()) {
+    if (trimBucketName().isEmpty()) {
       // If the staging location is empty, we don't have anything to verify; and we don't have any
       // interesting messaging.
-      messageTarget.clear();
       setPageComplete(true);
       return;
-    } else if (!bucketNameOk()) {
-      setPageComplete(false);
+    }
+
+    IStatus status = bucketNameStatus();
+    if (!status.isOK()) {
+      messageTarget.setError(status.getMessage());
+      return;
+    }
+
+    if (client == null) {
+      // We can't verify the staging location because we don't have a GCS client
       return;
     }
-    
     verifyJob = VerifyStagingLocationJob.create(client, stagingLocation);
     verifyJob.schedule(VERIFY_LOCATION_DELAY_MS);
     final ListenableFutureProxy<VerifyStagingLocationResult> resultFuture =
@@ -278,7 +283,7 @@ public void focusLost(FocusEvent event) {
       updateStagingLocations(getProject());
     }
   }
-  
+
   /**
    * Create a GCS bucket in the project specified in the project input at the location specified in
    * the staging location input.
@@ -302,7 +307,7 @@ public void widgetSelected(SelectionEvent event) {
       }
     }
   }
-  
+
   private void setPageComplete(boolean complete) {
     if (page != null) {
       page.setPageComplete(complete);
@@ -346,28 +351,26 @@ public void run() {
       }
     }
   }
-  
+
   private static final BucketNameValidator bucketNameValidator = new BucketNameValidator();
-  
-  private boolean bucketNameOk() {
+
+  private String trimBucketName() {
     String bucketName = stagingLocationInput.getText().trim();
     if (bucketName.toLowerCase(Locale.US).startsWith("gs://")) {
       bucketName = bucketName.substring(5);
     }
-    IStatus status = bucketNameValidator.validate(bucketName);
-    if (!status.isOK()) {
-      messageTarget.setError(status.getMessage());
-      setPageComplete(false);
-    }
-    boolean enabled = status.isOK() && !bucketName.isEmpty();
-    return enabled;
+    return bucketName;
   }
-  
+
+  private IStatus bucketNameStatus() {
+    return bucketNameValidator.validate(trimBucketName());
+  }
+
   private class EnableCreateButton implements ModifyListener {
 
     @Override
     public void modifyText(ModifyEvent event) {
-      boolean enabled = bucketNameOk();
+      boolean enabled = !trimBucketName().isEmpty() && bucketNameStatus().isOK();
       createButton.setEnabled(enabled);
     }
 

plugins/com.google.cloud.tools.eclipse.dataflow.ui/src/com/google/cloud/tools/eclipse/dataflow/ui/util/SelectFirstMatchingPrefixListener.java

@@ -17,16 +17,12 @@
 package com.google.cloud.tools.eclipse.dataflow.ui.util;
 
 import com.google.common.collect.ImmutableSortedSet;
-
+import java.util.SortedSet;
 import org.eclipse.swt.events.ModifyEvent;
 import org.eclipse.swt.events.ModifyListener;
 import org.eclipse.swt.graphics.Point;
 import org.eclipse.swt.widgets.Combo;
 
-import java.util.Collection;
-import java.util.LinkedHashSet;
-import java.util.SortedSet;
-
 /**
  * Select the first string that contains the current input as a prefix whenever input is provided
  * to the provided Combo.
@@ -37,13 +33,10 @@ public class SelectFirstMatchingPrefixListener implements ModifyListener {
 
   private SortedSet<String> contents;
 
-  private final Collection<OnCompleteListener> onCompleteListeners;
-
   public SelectFirstMatchingPrefixListener(Combo targetCombo) {
     this.targetCombo = targetCombo;
     lastString = "";
     contents = ImmutableSortedSet.of();
-    onCompleteListeners = new LinkedHashSet<>();
   }
 
   public void setContents(SortedSet<String> contents) {
@@ -54,60 +47,33 @@ public void setContents(SortedSet<String> contents) {
   public void modifyText(ModifyEvent event) {
     String lastAsOfEvent = lastString;
     String prefix = targetCombo.getText();
-    try {
-      lastString = prefix;
-      // If content was deleted, don't immediately replace it
-      if (prefix.length() < lastAsOfEvent.length()) {
-        return;
-      }
-      // Don't autofill if the user isn't at the end of the string
-      if (targetCombo.getCaretPosition() < prefix.length() || prefix.length() < 6) {
-        return;
-      }
-      SortedSet<String> matchesAndLater = contents.tailSet(prefix);
-      if (matchesAndLater.isEmpty()) {
-        return;
-      }
-      String firstMatch = matchesAndLater.first();
-      if (!firstMatch.startsWith(prefix) || firstMatch.equals(prefix)) {
-        return;
-      }
-      int comboIndex = contents.size() - matchesAndLater.size();
-      // Don't fire on this modification of the combo
-      targetCombo.removeModifyListener(this);
-      targetCombo.select(comboIndex);
-
-      // Select the text that was added
-      targetCombo.setSelection(new Point(prefix.length(), firstMatch.length()));
 
-      // Fire on later user modifications of the combo
-      targetCombo.addModifyListener(this);
-    } finally {
-      notifyListeners(targetCombo.getText());
+    lastString = prefix;
+    // If content was deleted, don't immediately replace it
+    if (prefix.length() < lastAsOfEvent.length()) {
+      return;
     }
-  }
-
-  /**
-   * Add a listener that is notified after the target combo is updated.
-   */
-  public void addOnCompleteListener(OnCompleteListener onComplete) {
-    onCompleteListeners.add(onComplete);
-  }
-
-  private void notifyListeners(String contents) {
-    for (OnCompleteListener listener : onCompleteListeners) {
-      listener.onComplete(contents);
+    // Don't autofill if the user isn't at the end of the string
+    if (targetCombo.getCaretPosition() < prefix.length() || prefix.length() < 6) {
+      return;
     }
-  }
+    SortedSet<String> matchesAndLater = contents.tailSet(prefix);
+    if (matchesAndLater.isEmpty()) {
+      return;
+    }
+    String firstMatch = matchesAndLater.first();
+    if (!firstMatch.startsWith(prefix) || firstMatch.equals(prefix)) {
+      return;
+    }
+    int comboIndex = contents.size() - matchesAndLater.size();
+    // Don't fire on this modification of the combo
+    targetCombo.removeModifyListener(this);
+    targetCombo.select(comboIndex);
+
+    // Select the text that was added
+    targetCombo.setSelection(new Point(prefix.length(), firstMatch.length()));
 
-  /**
-   * A listener that is notified when the {@link SelectFirstMatchingPrefixListener} completes with
-   * the contents of the Combo.
-   */
-  public interface OnCompleteListener {
-    /**
-     * @param contents the contents of the {@link SelectFirstMatchingPrefixListener} target.
-     */
-    void onComplete(String contents);
+    // Fire on later user modifications of the combo
+    targetCombo.addModifyListener(this);
   }
 }