Skip to content

Commit

Permalink
[JBIDE-22592] always fire change for same properties list instance (#…
Browse files Browse the repository at this point in the history 
…1261)
  • Loading branch information
@adietish
adietish committed Jul 20, 2016
1 parent 1be9820 commit 2a7bd80
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 32 deletions.
94 changes: 66 additions & 28 deletions .../openshift/internal/ui/wizard/newapp/fromtemplate/ApplicationSourceFromTemplateModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.stream.Collectors;

import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang.StringUtils;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.jobs.IJobChangeListener;
Expand Down Expand Up @@ -141,8 +141,11 @@ private void handleSelectedAppSource(PropertyChangeEvent evt) {
if(evt.getNewValue() instanceof IApplicationSource
&& ResourceKind.TEMPLATE.equals(((IApplicationSource) evt.getNewValue()).getKind())){
IApplicationSource source = (IApplicationSource) evt.getNewValue();
this.template = (ITemplate) source.getSource();
updateTemplateParameters(template);
ITemplate newTemplate = (ITemplate) source.getSource();
if (!Objects.equals(newTemplate, this.template)) {
this.template = newTemplate;
updateTemplateParameters(newTemplate);
}
}
}
private void handleEclipseProject(PropertyChangeEvent evt) {
Expand All @@ -165,7 +168,7 @@ private void updateTemplateParameters(ITemplate template) {
if (template == null) {
return;
}
setParameters(new ArrayList<>(template.getParameters().values()));
setParameters(template.getParameters().values());
setItems(template.getItems());
setLabels(template.getObjectLabels());
}
Expand All @@ -181,54 +184,60 @@ public boolean isParameterModified(IParameter param) {
}

@Override
public void setParameters(List<IParameter> parameters) {
public void setParameters(Collection<IParameter> parameters) {
List<IParameter> oldParameters = new ArrayList<>(this.parameters);
if (parameters == null) {
this.parameters.clear();
} else {
this.originalValueMap = toMap(parameters);
List<IParameter> newParameters = new ArrayList<>();
newParameters.addAll(injectProjectParameters(eclipseProject, parameters));
this.parameters.clear();
this.parameters.addAll(newParameters);
Collections.sort(this.parameters, new TemplateParameterViewerUtils.ParameterNameComparator());
}
firePropertyChange(PROPERTY_PARAMETERS, oldParameters, this.parameters);

// update selected parameter
setSelectedParameter(getSelectedParameterOrDefault());
}

private Map<String, String> toMap(Collection<IParameter> parameters) {
Map<String, String> paramsMap = new HashMap<>();
if (parameters != null) {
parameters.forEach(p -> paramsMap.put(p.getName(), p.getValue()));
}
originalValueMap = paramsMap;
firePropertyChange(PROPERTY_PARAMETERS, this.parameters, this.parameters = injectProjectParameters(this.eclipseProject, parameters));
return paramsMap;
}

private static List<IParameter> injectProjectParameters(org.eclipse.core.resources.IProject project, List<IParameter> originalParameters) {
private Collection<IParameter> injectProjectParameters(org.eclipse.core.resources.IProject project, Collection<IParameter> originalParameters) {
if (originalParameters == null || originalParameters.isEmpty()) {
return originalParameters;
}
Map<String, String> projectParams = getProjectParameters(project);

List<IParameter> newParameters = originalParameters.stream().map(p -> {
IParameter clone = p.clone();
String value = projectParams.get(clone.getName());
originalParameters.forEach(p -> {
String value = projectParams.get(p.getName());
if (value != null) {
clone.setValue(value);
p.setValue(value);
}
return clone;
}).collect(Collectors.toList());
});

return newParameters;
return originalParameters;
}

private static Map<String, String> getProjectParameters(org.eclipse.core.resources.IProject project) {
private Map<String, String> getProjectParameters(org.eclipse.core.resources.IProject project) {
if(project == null) {
return Collections.emptyMap();
}

Map<String,String> projectParams = new HashMap<>();
String gitRepo = null;
try {
gitRepo = StringUtils.defaultString(EGitUtils.getDefaultRemoteRepo(project));
} catch (CoreException e) {
throw new OpenShiftException(e, NLS.bind("Could not determine the default remote Git repository for \"{0}\"", project.getName()));
}
String gitRepo = getGitRepo(project);
if (gitRepo != null) {
projectParams.put(PARAMETER_SOURCE_REPOSITORY_URL, gitRepo);
projectParams.put(PARAMETER_GIT_URI, gitRepo);//legacy key

String branch;
try {
branch = StringUtils.defaultString(EGitUtils.getCurrentBranch(project));
} catch (CoreException e) {
throw new OpenShiftException(e, NLS.bind("Could not determine the default Git branch for \"{0}\"", project.getName()));
}
String branch = getGitBranch(project);
projectParams.put("SOURCE_REPOSITORY_REF", branch);
projectParams.put("GIT_REF", branch);//legacy key

Expand All @@ -242,11 +251,40 @@ private static Map<String, String> getProjectParameters(org.eclipse.core.resourc
return projectParams;
}

private String getGitBranch(org.eclipse.core.resources.IProject project) {
try {
return StringUtils.defaultString(EGitUtils.getCurrentBranch(project));
} catch (CoreException e) {
throw new OpenShiftException(e, NLS.bind("Could not determine the default Git branch for \"{0}\"", project.getName()));
}
}

private String getGitRepo(org.eclipse.core.resources.IProject project) {
try {
return StringUtils.defaultString(EGitUtils.getDefaultRemoteRepo(project));
} catch (CoreException e) {
throw new OpenShiftException(e, NLS.bind("Could not determine the default remote Git repository for \"{0}\"", project.getName()));
}
}

@Override
public IParameter getSelectedParameter() {
return this.selectedParameter;
}

private IParameter getSelectedParameterOrDefault() {
if (selectedParameter == null
|| !parameters.contains(selectedParameter)) {
if (CollectionUtils.isEmpty(parameters)) {
return null;
} else {
return parameters.get(0);
}
} else {
return selectedParameter;
}
}

@Override
public void setSelectedParameter(IParameter parameter) {
firePropertyChange(PROPERTY_SELECTED_PARAMETER, this.selectedParameter, this.selectedParameter = parameter);
Expand Down
3 changes: 2 additions & 1 deletion .../tools/openshift/internal/ui/wizard/newapp/fromtemplate/ITemplateParametersPageModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
******************************************************************************/
package org.jboss.tools.openshift.internal.ui.wizard.newapp.fromtemplate;

import java.util.Collection;
import java.util.List;

import com.openshift.restclient.model.template.IParameter;
Expand All @@ -34,7 +35,7 @@ public interface ITemplateParametersPageModel {
* Set the list of template parameters
* @param parameters
*/
void setParameters(List<IParameter> parameters);
void setParameters(Collection<IParameter> parameters);

/**
* Get the selected parameter
Expand Down
16 changes: 14 additions & 2 deletions .../tools/openshift/internal/ui/wizard/newapp/fromtemplate/TemplateParameterViewerUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import static org.apache.commons.lang.StringUtils.defaultIfBlank;
import static org.apache.commons.lang.StringUtils.isNotBlank;

import java.util.Comparator;

import org.eclipse.jface.viewers.Viewer;
import org.eclipse.jface.viewers.ViewerComparator;

Expand Down Expand Up @@ -42,11 +44,21 @@ public static String getValueLabel(IParameter parameter) {
*/
public static class ParameterNameViewerComparator extends ViewerComparator {

Comparator<IParameter> comparator = new ParameterNameComparator();

@Override
public int compare(Viewer viewer, Object e1, Object e2) {
IParameter first = (IParameter) e1;
IParameter other = (IParameter) e2;
return first.getName().compareTo(other.getName());
IParameter second = (IParameter) e2;
return comparator.compare(first, second);
}
}

public static class ParameterNameComparator implements Comparator<IParameter> {

@Override
public int compare(IParameter parameter1, IParameter parameter2) {
return parameter1.getName().compareTo(parameter2.getName());
}
}

Expand Down
2 changes: 1 addition & 1 deletion .../jboss/tools/openshift/internal/ui/wizard/newapp/fromtemplate/TemplateParametersPage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ protected void doCreateControls(Composite container, DataBindingContext dbc) {
this.viewer = createTable(tableContainer, parametersObservable, dbc);
GridDataFactory.fillDefaults()
.span(1, 5).align(SWT.FILL, SWT.FILL).grab(true, true).hint(500, 300).applyTo(tableContainer);
viewer.setInput(parametersObservable);
IObservableValue<IParameter> selectedParameter =
BeanProperties.value(ITemplateParametersPageModel.PROPERTY_SELECTED_PARAMETER).observe(model);
ValueBindingBuilder.bind(ViewerProperties.singleSelection().observe(viewer))
.to(selectedParameter)
.in(dbc);
viewer.setInput(parametersObservable);
viewer.addDoubleClickListener(onDoubleClick());

// edit button
Expand Down
48 changes: 48 additions & 0 deletions ...org/jboss/tools/openshift/test/ui/application/ApplicationSourceFromTemplateModelTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@
******************************************************************************/
package org.jboss.tools.openshift.test.ui.application;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.*;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand All @@ -21,6 +23,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;

Expand Down Expand Up @@ -96,6 +99,51 @@ public void updateParameterValueShouldUpdateTheParameterValue() {
verify(param).setValue(eq("abc123"));
}

@Test
public void should_return_first_parameter_as_selected_parameter_when_new_parameters_are_set() {
// given
assertThat(model.getSelectedParameter()).isNull();
List<IParameter> parameters = Arrays.asList(
mockParameter("n1", "v1"),
mockParameter("n2", "v2"),
mockParameter("n3", "v3")
);
// when
model.setParameters(parameters);
// then
assertThat(model.getSelectedParameter()).isNotNull();
assertThat(model.getSelectedParameter().getName()).isEqualTo("n1");
}

@Test
public void should_reset_selected_parameter_to_first_parameter_as_selected_parameter_when_new_parameters_are_set() {
// given
List<IParameter> parameters = Arrays.asList(
mockParameter("n1", "v1"),
mockParameter("n2", "v2"),
mockParameter("n3", "v3")
);
model.setParameters(parameters);
model.setSelectedParameter(parameters.get(1));
assertThat(model.getSelectedParameter().getName()).isEqualTo("n2");
// when
model.setParameters(Arrays.asList(
mockParameter("n10", "v10"),
mockParameter("n20", "v20"),
mockParameter("n30", "v30")
));
// then
assertThat(model.getSelectedParameter()).isNotNull();
assertThat(model.getSelectedParameter().getName()).isEqualTo("n10");
}

private IParameter mockParameter(String name, String value) {
IParameter parameter = mock(IParameter.class);
doReturn(name).when(parameter).getName();
doReturn(value).when(parameter).getValue();
return parameter;
}

@Test
public void getParametersShouldReturnAParameterMapWhenTemplateIsNotNull() {
Map<String, IParameter> parameters = givenTheTemplateHasParameters();
Expand Down

0 comments on commit 2a7bd80

Please sign in to comment.