Validate Project Selector (#1410)

* Validate Project Selector * Create and share one IValidator instance. * Remove ununsed Import-Package * Clean up code * checkArgument (checkState x) / simplify validator * Remove hamcrest
GoogleCloudPlatform · Feb 15, 2017 · b024216 · b024216
1 parent b2d0e43
commit b024216
Show file tree

Hide file tree

Showing 10 changed files with 67 additions and 145 deletions.
diff --git a/...om/google/cloud/tools/eclipse/appengine/deploy/ui/StandardDeployPreferencesPanelTest.java b/...om/google/cloud/tools/eclipse/appengine/deploy/ui/StandardDeployPreferencesPanelTest.java
@@ -17,7 +17,6 @@
 package com.google.cloud.tools.eclipse.appengine.deploy.ui;
 
 import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -80,13 +79,6 @@ public void setUp() throws Exception {
     when(account2.getOAuth2Credential()).thenReturn(mock(Credential.class));
   }
 
-  @Test
-  public void testHasSelection() {
-    StandardDeployPreferencesPanel deployPanel = new StandardDeployPreferencesPanel(
-        parent, project, loginService, layoutChangedHandler, true, projectRepository);
-    assertFalse(deployPanel.hasSelection());
-  }
-
   @Test
   public void testSelectSingleAccount() {
     when(loginService.getAccounts()).thenReturn(new HashSet<>(Arrays.asList(account1)));
@@ -148,7 +140,7 @@ public void testProjectSavedInPreferencesSelected() throws ProjectRepositoryExce
     fail("Did not find ProjectSelector widget");
   }
 
-  private void initializeProjectRepository(ProjectRepository projectRepository) 
+  private void initializeProjectRepository(ProjectRepository projectRepository)
       throws ProjectRepositoryException {
     GcpProject project1 = new GcpProject("Project1", "projectId1");
     GcpProject project2 = new GcpProject("Project2", "projectId2");

diff --git a/...oy.ui/src/com/google/cloud/tools/eclipse/appengine/deploy/ui/DeployPreferencesDialog.java b/...oy.ui/src/com/google/cloud/tools/eclipse/appengine/deploy/ui/DeployPreferencesDialog.java
@@ -76,9 +76,7 @@ protected Control createContents(Composite parent) {
       setTitleImage(titleImage);
     }
 
-    Button deployButton = getButton(IDialogConstants.OK_ID);
-    deployButton.setText(Messages.getString("deploy"));
-    deployButton.setEnabled(false);
+    getButton(IDialogConstants.OK_ID).setText(Messages.getString("deploy"));
 
     // TitleAreaDialogSupport does not validate initially, let's trigger validation this way
     content.getDataBindingContext().updateTargets();
@@ -109,7 +107,7 @@ protected Control createDialogArea(final Composite parent) {
           @Override
           public int getMessageType(ValidationStatusProvider statusProvider) {
             int type = super.getMessageType(statusProvider);
-            setValid(type != IMessageProvider.ERROR && content.hasSelection());
+            setValid(type != IMessageProvider.ERROR);
             return type;
           }
         });
@@ -180,7 +178,7 @@ public boolean isHelpAvailable() {
   private void setValid(boolean isValid) {
     Button deployButton = getButton(IDialogConstants.OK_ID);
     if (deployButton != null) {
-      deployButton.setEnabled(isValid && content.hasSelection());
+      deployButton.setEnabled(isValid);
     }
   }
 

diff --git a/...rc/com/google/cloud/tools/eclipse/appengine/deploy/ui/StandardDeployPreferencesPanel.java b/...rc/com/google/cloud/tools/eclipse/appengine/deploy/ui/StandardDeployPreferencesPanel.java
@@ -20,15 +20,17 @@
 import com.google.cloud.tools.eclipse.login.IGoogleLoginService;
 import com.google.cloud.tools.eclipse.login.ui.AccountSelector;
 import com.google.cloud.tools.eclipse.login.ui.AccountSelectorObservableValue;
-import com.google.cloud.tools.eclipse.projectselector.ProjectRepository;
 import com.google.cloud.tools.eclipse.projectselector.GcpProject;
+import com.google.cloud.tools.eclipse.projectselector.ProjectRepository;
 import com.google.cloud.tools.eclipse.projectselector.ProjectRepositoryException;
 import com.google.cloud.tools.eclipse.projectselector.ProjectSelector;
 import com.google.cloud.tools.eclipse.ui.util.FontUtil;
 import com.google.cloud.tools.eclipse.ui.util.databinding.BucketNameValidator;
+import com.google.cloud.tools.eclipse.ui.util.databinding.ProjectSelectorValidator;
 import com.google.cloud.tools.eclipse.ui.util.databinding.ProjectVersionValidator;
 import com.google.cloud.tools.eclipse.util.status.StatusUtil;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import java.util.Collections;
 import java.util.List;
@@ -44,6 +46,7 @@
 import org.eclipse.core.databinding.observable.list.IObservableList;
 import org.eclipse.core.databinding.observable.value.IObservableValue;
 import org.eclipse.core.databinding.observable.value.WritableValue;
+import org.eclipse.core.databinding.validation.IValidator;
 import org.eclipse.core.databinding.validation.MultiValidator;
 import org.eclipse.core.databinding.validation.ValidationStatus;
 import org.eclipse.core.resources.IProject;
@@ -181,11 +184,21 @@ public Object convert(Object expectedEmail) {
   }
 
   private void setupProjectIdDataBinding(DataBindingContext context) {
-    IViewerObservableValue projectList = ViewerProperties.singleSelection().observe(projectSelector.getViewer());
+    IViewerObservableValue projectList =
+        ViewerProperties.singleSelection().observe(projectSelector.getViewer());
     IObservableValue projectIdModel = PojoProperties.value("projectId").observe(model);
-    context.bindValue(projectList, projectIdModel,
-                      new UpdateValueStrategy().setConverter(new GcpProjectToProjectIdConverter()),
-                      new UpdateValueStrategy().setConverter(new ProjectIdToGcpProjectConverter()));
+
+    UpdateValueStrategy gcpProjectToProjectId =
+        new UpdateValueStrategy().setConverter(new GcpProjectToProjectIdConverter());
+    UpdateValueStrategy projectIdToGcpProject =
+        new UpdateValueStrategy().setConverter(new ProjectIdToGcpProjectConverter());
+    if (requireValues) {
+      IValidator validator = new ProjectSelectorValidator();
+      gcpProjectToProjectId.setAfterConvertValidator(validator);
+      projectIdToGcpProject.setAfterGetValidator(validator);
+    }
+
+    context.bindValue(projectList, projectIdModel, gcpProjectToProjectId, projectIdToGcpProject);
   }
 
   private void setupProjectVersionDataBinding(DataBindingContext context) {
@@ -386,6 +399,8 @@ public Object convert(Object fromObject) {
       if (fromObject == null) {
         return null;
       }
+
+      Preconditions.checkArgument(fromObject instanceof String);
       try {
         return projectRepository.getProject(accountSelector.getSelectedCredential(),
                                            (String) fromObject);
@@ -406,6 +421,8 @@ public Object convert(Object fromObject) {
       if (fromObject == null) {
         return null;
       }
+
+      Preconditions.checkArgument(fromObject instanceof GcpProject);
       return ((GcpProject) fromObject).getId();
     }
   }
@@ -489,8 +506,4 @@ public void setFont(Font font) {
     expandableComposite.setFont(font);
     FontUtil.convertFontToBold(expandableComposite);
   }
-
-  boolean hasSelection() {
-    return projectSelector.hasSelection();
-  }
 }
diff --git a/...selector.test/src/com/google/cloud/tools/eclipse/projectselector/ProjectSelectorTest.java b/...selector.test/src/com/google/cloud/tools/eclipse/projectselector/ProjectSelectorTest.java
@@ -18,8 +18,6 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 
 import com.google.cloud.tools.eclipse.test.util.ui.ShellTestResource;
 import java.util.Arrays;
@@ -64,16 +62,14 @@ public void testProjectsAreSortedAlphabetically() throws Exception {
   public void testSetProjectMaintainsSelection() {
     List<GcpProject> projects = getUnsortedProjectList();
     GcpProject selectedProject = projects.get(3);
-    
+
     ProjectSelector projectSelector = new ProjectSelector(shellResource.getShell());
     projectSelector.setProjects(projects);
-    assertFalse(projectSelector.hasSelection());
     projectSelector.getViewer().setSelection(new StructuredSelection(selectedProject));
     projectSelector.setProjects(projects.subList(2, projects.size()));
 
     IStructuredSelection selection = projectSelector.getViewer().getStructuredSelection();
     assertThat(selection.size(), is(1));
-    assertTrue(projectSelector.hasSelection());
     assertThat((GcpProject) selection.getFirstElement(), is(selectedProject));
   }
 

diff --git a/...e.projectselector/src/com/google/cloud/tools/eclipse/projectselector/ProjectSelector.java b/...e.projectselector/src/com/google/cloud/tools/eclipse/projectselector/ProjectSelector.java
@@ -73,9 +73,4 @@ public void setProjects(List<GcpProject> projects) {
     }
     tableViewer.setSelection(selection);
   }
-
-  public boolean hasSelection() {
-    ISelection selection = tableViewer.getSelection();
-    return !selection.isEmpty();
-  }
 }
diff --git a/...t/src/com/google/cloud/tools/eclipse/ui/util/databinding/ProjectIdInputValidatorTest.java b/...t/src/com/google/cloud/tools/eclipse/ui/util/databinding/ProjectIdInputValidatorTest.java
diff --git a/.../src/com/google/cloud/tools/eclipse/ui/util/databinding/ProjectSelectorValidatorTest.java b/.../src/com/google/cloud/tools/eclipse/ui/util/databinding/ProjectSelectorValidatorTest.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Licensed 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 com.google.cloud.tools.eclipse.ui.util.databinding;
+
+import static org.junit.Assert.assertEquals;
+
+import org.eclipse.core.runtime.IStatus;
+import org.junit.Test;
+
+public class ProjectSelectorValidatorTest {
+
+  @Test
+  public void testValidate_nullString() {
+    assertEquals(IStatus.ERROR, new ProjectSelectorValidator().validate(null).getSeverity());
+  }
+
+  @Test
+  public void testValidate_emptyString() {
+    assertEquals(IStatus.ERROR, new ProjectSelectorValidator().validate("").getSeverity());
+  }
+}
diff --git a/plugins/com.google.cloud.tools.eclipse.ui.util/META-INF/MANIFEST.MF b/plugins/com.google.cloud.tools.eclipse.ui.util/META-INF/MANIFEST.MF
@@ -13,7 +13,6 @@ Require-Bundle: org.eclipse.ui.workbench,
 Import-Package: com.google.cloud.tools.eclipse.ui.util,
  com.google.cloud.tools.eclipse.util,
  com.google.cloud.tools.eclipse.util.status,
- com.google.cloud.tools.project;version="0.1.9",
  com.google.common.annotations;version="[20.0.0,21.0.0)",
  com.google.common.base;version="[20.0.0,21.0.0)",
  org.eclipse.core.commands,

diff --git a/.../databinding/ProjectIdInputValidator.java → ...databinding/ProjectSelectorValidator.java b/.../databinding/ProjectIdInputValidator.java → ...databinding/ProjectSelectorValidator.java
@@ -17,36 +17,17 @@
 package com.google.cloud.tools.eclipse.ui.util.databinding;
 
 import com.google.cloud.tools.eclipse.ui.util.Messages;
-import com.google.cloud.tools.project.ProjectIdValidator;
 import org.eclipse.core.databinding.validation.IValidator;
 import org.eclipse.core.databinding.validation.ValidationStatus;
 import org.eclipse.core.runtime.IStatus;
 
-public class ProjectIdInputValidator implements IValidator {
-  private boolean requireProjectId = true;
-
-  public ProjectIdInputValidator(boolean requireProjectId) {
-    this.requireProjectId = requireProjectId;
-  }
+public class ProjectSelectorValidator implements IValidator {
 
   @Override
   public IStatus validate(Object input) {
-    if (!(input instanceof String)) {
-      return ValidationStatus.error(Messages.getString("project.id.invalid")); //$NON-NLS-1$
-    }
-    String value = (String) input;
-    return validateString(value);
-  }
-
-  private IStatus validateString(String value) {
-    if (value.isEmpty()) {
-      return requireProjectId ?
-          ValidationStatus.error(Messages.getString("project.id.empty")) : //$NON-NLS-1$
-          ValidationStatus.ok();
-    } else if (ProjectIdValidator.validate(value)) {
-      return ValidationStatus.ok();
-    } else {
-      return ValidationStatus.error(Messages.getString("project.id.invalid")); //$NON-NLS-1$
+    if (input == null || ((String) input).isEmpty()) {
+      return ValidationStatus.error(Messages.getString("project.not.selected")); //$NON-NLS-1$
     }
+    return ValidationStatus.ok();
   }
 }
diff --git a/...loud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/messages.properties b/...loud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/messages.properties
@@ -1,5 +1,4 @@
 bucket.name.invalid=Invalid bucket name
-project.id.invalid=Invalid project ID
-project.id.empty=Empty project ID
+project.not.selected=Project ID not selected
 version.invalid=Version must contain only lower-case letters, numbers, and hyphens
 version.reserved=Version uses reserved form