Bug 475465 - NPE when image contains in tag in the Search dialog
Fixing the case where no value was put in the queue if a server
error occurred, which resulted in UI blocked for 10s (until
timeout) and then an NPE.
Added databinding validation support to validate the given
search term against a regex (as found in the CLI help)
Change-Id: Ia3eb1836904ce036ca3368043582b8979d3640d1
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
Reviewed-on: https://git.eclipse.org/r/54227
Tested-by: Hudson CI
Reviewed-by: Roland Grunberg <rgrunber@redhat.com>
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ImageSearchPage.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ImageSearchPage.java
index 98cf5e5..52a5e18 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ImageSearchPage.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ImageSearchPage.java
@@ -12,22 +12,32 @@
package org.eclipse.linuxtools.internal.docker.ui.wizards;
import java.lang.reflect.InvocationTargetException;
+import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
+import org.eclipse.core.databinding.AggregateValidationStatus;
import org.eclipse.core.databinding.DataBindingContext;
+import org.eclipse.core.databinding.UpdateValueStrategy;
import org.eclipse.core.databinding.beans.BeanProperties;
import org.eclipse.core.databinding.beans.PojoProperties;
import org.eclipse.core.databinding.observable.list.IObservableList;
import org.eclipse.core.databinding.observable.value.IObservableValue;
import org.eclipse.core.databinding.observable.value.IValueChangeListener;
import org.eclipse.core.databinding.observable.value.ValueChangeEvent;
+import org.eclipse.core.databinding.validation.IValidator;
+import org.eclipse.core.databinding.validation.ValidationStatus;
import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Status;
+import org.eclipse.jface.databinding.swt.ISWTObservableValue;
import org.eclipse.jface.databinding.swt.WidgetProperties;
import org.eclipse.jface.databinding.viewers.ObservableListContentProvider;
import org.eclipse.jface.databinding.viewers.ViewerProperties;
+import org.eclipse.jface.databinding.wizard.WizardPageSupport;
import org.eclipse.jface.layout.GridDataFactory;
import org.eclipse.jface.layout.GridLayoutFactory;
import org.eclipse.jface.operation.IRunnableWithProgress;
@@ -78,7 +88,6 @@
super("ImageSearchPage", //$NON-NLS-1$
WizardMessages.getString("ImageSearchPage.title"), //$NON-NLS-1$
SWTImagesFactory.DESC_BANNER_REPOSITORY);
- setMessage(WizardMessages.getString("ImageSearchPage.title")); //$NON-NLS-1$
this.model = model;
}
@@ -170,23 +179,16 @@
final IObservableValue observableTermModel = BeanProperties
.value(ImageSearchModel.class, ImageSearchModel.TERM)
.observe(model);
- ctx.bindValue(
- WidgetProperties.text(SWT.Modify).observe(searchImageText),
- observableTermModel);
- // enable/disable the search button
- observableTermModel.addValueChangeListener(new IValueChangeListener() {
+ final UpdateValueStrategy strategy = new UpdateValueStrategy();
+ strategy.setBeforeSetValidator(new SearchTermValidator());
- @Override
- public void handleValueChange(final ValueChangeEvent event) {
- final String term = (String) event.getObservableValue()
- .getValue();
- if (term.isEmpty()) {
- searchImageButton.setEnabled(false);
- } else {
- searchImageButton.setEnabled(true);
- }
- }
- });
+ final ISWTObservableValue imageSearchTextObservable = WidgetProperties
+ .text(SWT.Modify).observe(searchImageText);
+ ctx.bindValue(imageSearchTextObservable, observableTermModel, strategy,
+ null);
+ // enable/disable the search button
+ imageSearchTextObservable
+ .addValueChangeListener(onTermValueChanged(searchImageButton));
// observe the viewer content
searchResultTableViewer
.setContentProvider(new ObservableListContentProvider());
@@ -211,6 +213,8 @@
ctx.bindValue(WidgetProperties.text().observe(selectedImageDescription),
observableSelectedImageDescription);
searchImageText.setFocus();
+ // attach the Databinding context status to this wizard page.
+ WizardPageSupport.create(this, this.ctx);
setControl(container);
}
@@ -229,6 +233,26 @@
return viewerColumn;
}
+ private IValueChangeListener onTermValueChanged(
+ final Button searchImageButton) {
+ return new IValueChangeListener() {
+
+ @Override
+ public void handleValueChange(final ValueChangeEvent event) {
+ final String searchTerm = (String) event.getObservableValue()
+ .getValue();
+ final IStatus status = AggregateValidationStatus
+ .getStatusMaxSeverity(
+ ctx.getValidationStatusProviders());
+ if (searchTerm.isEmpty() || !status.isOK()) {
+ searchImageButton.setEnabled(false);
+ } else {
+ searchImageButton.setEnabled(true);
+ }
+ }
+ };
+ }
+
private TraverseListener onSearchImageTextTraverse() {
return new TraverseListener() {
@@ -246,8 +270,12 @@
@Override
public void keyReleased(final KeyEvent event) {
- if (event.character == SWT.CR
- && !ImageSearchPage.this.model.getTerm().isEmpty()) {
+ final IStatus status = AggregateValidationStatus
+ .getStatusMaxSeverity(
+ ctx.getValidationStatusProviders());
+ final String searchTerm = ImageSearchPage.this.model.getTerm();
+ if (event.character == SWT.CR && !searchTerm.isEmpty()
+ && status.isOK()) {
searchImages();
}
}
@@ -285,6 +313,8 @@
searchResultQueue.offer(searchResults);
} catch (DockerException e) {
Activator.log(e);
+ searchResultQueue.offer(
+ new ArrayList<IDockerImageSearchResult>());
}
monitor.done();
}
@@ -328,6 +358,28 @@
return this.model.getSelectedImage() != null;
}
+ static class SearchTermValidator implements IValidator {
+
+ private static final Pattern termPattern = Pattern
+ .compile("[a-z0-9]+([._-][a-z0-9]+)*"); //$NON-NLS-1$
+
+ @Override
+ public IStatus validate(final Object value) {
+ final String term = (String) value;
+ if (term == null || term.isEmpty()) {
+ return ValidationStatus.info(WizardMessages
+ .getString("ImageSearchPage.description")); //$NON-NLS-1$
+ } else if (termPattern.matcher(term).matches()) {
+ return Status.OK_STATUS;
+ } else {
+ return ValidationStatus.error(WizardMessages.getFormattedString(
+ "ImageSearchPage.term.invalidformat", //$NON-NLS-1$
+ termPattern.pattern()));
+ }
+ }
+
+ }
+
static class ImageNameColumnLabelProvider extends ColumnLabelProvider {
@Override
@@ -401,7 +453,6 @@
}
abstract boolean doPaint(final Object element);
-
}
}
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/WizardMessages.properties b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/WizardMessages.properties
index cc4f63d..5a5733c 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/WizardMessages.properties
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/WizardMessages.properties
@@ -170,6 +170,8 @@
ImageSearch.title=Search and pull a Docker image
ImageSearchPage.title=Search the Docker Registry for images
+ImageSearchPage.description=Input an image name as the search term
+ImageSearchPage.term.invalidformat=Search term must match {0}
ImageSearchPage.imageLabel=Image:
ImageSearchPage.search=Search
ImageSearchPage.searchResultLabel=Matching images
