Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

JBIDE-24768 - ensure changes to extended properties take effect after… #1531

Merged
merged 2 commits into from Jul 27, 2017
Merged

JBIDE-24768 - ensure changes to extended properties take effect after… #1531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
65d3425
JBIDE-24768 - ensure changes to extended properties take effect after…
robstryker Jul 26, 2017
e3553f4
JBIDE-24770 - rename a job
robstryker Jul 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ls/openshift/cdk/server/core/internal/listeners/ConfigureDependentFrameworksListener.java
View file
Open in desktop
Expand Up @@ -38,7 +38,7 @@ public void serverChanged(final ServerEvent event) {
}

private void scheduleConfigureFrameworksJob(final ServerEvent event) {
new Job("Loading service-manager to configure additional frameworks that CDK depends on.") {
new Job("Inspecting CDK environment...") {
@Override
protected IStatus run(IProgressMonitor monitor) {
try {
Expand Down
9 changes: 9 additions & 0 deletions ...s/tools/openshift/internal/common/ui/connection/AdvancedConnectionEditorsStackedView.java
View file
Open in desktop
Expand Up @@ -43,4 +43,13 @@ private Collection<IAdvancedConnectionPropertiesEditor> getEditors() {
return ExtensionUtils.getExtensions(EXTENSION, ATTRIBUTE_CLASS);
}

public void saveChanges(ConnectionWizardPageModel pageModel) {
IAdvancedConnectionPropertiesEditor[] ed = editors.toArray(new IAdvancedConnectionPropertiesEditor[editors.size()]);
for( int i = 0; i < ed.length; i++ ) {
ed[i].saveChanges(pageModel);
}
}



}
3 changes: 3 additions & 0 deletions ....ui/src/org/jboss/tools/openshift/internal/common/ui/connection/ConnectionWizardPage.java
View file
Open in desktop
Expand Up @@ -386,6 +386,9 @@ public boolean connect() {
connectJob, new DelegatingProgressMonitor(), getContainer(), getDatabindingContext());
boolean connected = JobUtils.isOk(connectJob.getConnectionStatus());
if (connected) {

// update the connection and save it
advConnectionEditors.saveChanges(pageModel);
boolean result = pageModel.saveConnection();
if(result) {
SecureStoreException e = pageModel.getRecentSecureStoreException();
Expand Down
2 changes: 2 additions & 0 deletions ...ss/tools/openshift/internal/common/ui/connection/IAdvancedConnectionPropertiesEditor.java
View file
Open in desktop
Expand Up @@ -14,4 +14,6 @@

public interface IAdvancedConnectionPropertiesEditor extends IDetailView {

void saveChanges(ConnectionWizardPageModel pageModel);

}
1 change: 1 addition & 0 deletions ....jboss.tools.openshift.core/src/org/jboss/tools/openshift/core/connection/Connection.java
View file
Open in desktop
Expand Up @@ -402,6 +402,7 @@ public void update(IConnection connection) {
this.rememberPassword = otherConnection.rememberPassword;
this.tokenLoaded = otherConnection.tokenLoaded;
this.rememberPassword = otherConnection.rememberPassword;
this.extendedProperties = otherConnection.getExtendedProperties();

IAuthorizationContext otherContext = otherConnection.client.getAuthorizationContext();
IAuthorizationContext context = this.client.getAuthorizationContext();
Expand Down
201 changes: 107 additions & 94 deletions ...src/org/jboss/tools/openshift/internal/ui/wizard/connection/AdvancedConnectionEditor.java
View file
Open in desktop
Expand Up @@ -49,7 +49,6 @@
import org.jboss.tools.openshift.core.connection.Connection;
import org.jboss.tools.openshift.core.connection.ConnectionFactory;
import org.jboss.tools.openshift.core.preferences.OpenShiftCorePreferences;
import org.jboss.tools.openshift.internal.common.core.OpenShiftCommonCoreActivator;
import org.jboss.tools.openshift.internal.common.ui.connection.ConnectionWizardPageModel;
import org.jboss.tools.openshift.internal.common.ui.connection.ConnectionWizardPageModel.IConnectionAdvancedPropertiesProvider;
import org.jboss.tools.openshift.internal.common.ui.connection.IAdvancedConnectionPropertiesEditor;
Expand Down Expand Up @@ -225,72 +224,79 @@ protected GridLayoutFactory adjustAdvancedCompositeLayout(GridLayoutFactory grid


private void validateOCLocation(String location, boolean override) {
kickOCLocationValidation(location, override);
if( override )
kickOCLocationValidation(location);
else
decoration.hide();
}


private class OCVersionUIJob extends UIUpdatingJob {

private Version version;
private String error;
private String location;
public OCVersionUIJob(String location) {
super("Checking oc binary...");
this.location = location;
}

@Override
protected IStatus run(IProgressMonitor monitor) {

if (StringUtils.isBlank(location)) {
error = "OC Location cannot be empty";
} else {
File file = new File(location);
// Error messages have to be set to field editor, not directly to the
// page.
if (!file.exists()) {
error = (NLS.bind("{0} was not found.", file));
} else if (!file.canExecute()) {
error = (NLS.bind("{0} does not have execute permissions.", file));
}
}

if( error == null ) {
version = new OCBinaryVersionValidator(location).getVersion(monitor);
}
if (monitor.isCanceled()) {
return Status.CANCEL_STATUS;
}
return Status.OK_STATUS;
}

@Override
protected IStatus updateUI(IProgressMonitor monitor) {
if( error == null ) {
if (Version.emptyVersion.equals(version)) {
error = ("Could not determine your OpenShift client version");
} else if( !OCBinaryVersionValidator.isCompatibleForPublishing(version)) {
error = NLS.bind("OpenShift client version 1.1.1 or higher is required to avoid rsync issues.", version);
}
}

// now we have an error string
if( error == null ) {
// hide decorator
decoration.hide();
} else {
// show decorator, set message to error
decoration.setDescriptionText(error);
decoration.show();
}

return Status.OK_STATUS;
}
}

private UIUpdatingJob versionVerificationJob;
private void kickOCLocationValidation(String location, boolean override) {
if( !override ) {
return;
}

private void kickOCLocationValidation(String location) {
if( versionVerificationJob != null ) {
versionVerificationJob.cancel();
}

versionVerificationJob = new UIUpdatingJob("Checking oc binary...") {

private Version version;
private String error;

@Override
protected IStatus run(IProgressMonitor monitor) {

if (StringUtils.isBlank(location)) {
error = "OC Location cannot be empty";
} else {
File file = new File(location);
// Error messages have to be set to field editor, not directly to the
// page.
if (!file.exists()) {
error = (NLS.bind("{0} was not found.", file));
} else if (!file.canExecute()) {
error = (NLS.bind("{0} does not have execute permissions.", file));
}
}

if( error == null ) {
version = new OCBinaryVersionValidator(location).getVersion(monitor);
}
if (monitor.isCanceled()) {
return Status.CANCEL_STATUS;
}
return Status.OK_STATUS;
}

@Override
protected IStatus updateUI(IProgressMonitor monitor) {
if( error == null ) {
if (Version.emptyVersion.equals(version)) {
error = ("Could not determine your OpenShift client version");
} else if( !OCBinaryVersionValidator.isCompatibleForPublishing(version)) {
error = NLS.bind("OpenShift client version 1.1.1 or higher is required to avoid rsync issues.", version);
}
}

// now we have an error string
if( error == null ) {
// hide decorator
decoration.hide();
} else {
// show decorator, set message to error
decoration.setDescriptionText(error);
decoration.show();
}

return Status.OK_STATUS;
}
};
versionVerificationJob = new OCVersionUIJob(location);
versionVerificationJob.schedule();
}

Expand Down Expand Up @@ -343,9 +349,17 @@ public boolean isViewFor(Object object) {
return object instanceof ConnectionFactory;
}

private Connection getConnection() {
Map<String, Object> map = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an instance variable in the middle of the class? not helpful if you want to understand a class that has it's state spread across various places

Copy link
Member

@adietish adietish Jul 31, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calling this temporary storage for extended properties "map" isnt helpful either.
Why store those and create an additional "linked" state anyway?

public Map<String, Object> getExtendedProperties() {
if( map != null ) {
return map;
}
IConnection connection = pageModel.getSelectedConnection();
return connection instanceof Connection ? (Connection) connection : null;
if( connection != null ) {
map = ((Connection)connection).getExtendedProperties();
return map;
}
return null;
}

class AdvancedConnectionEditorModel extends ObservablePojo{
Expand All @@ -358,83 +372,74 @@ class AdvancedConnectionEditorModel extends ObservablePojo{
static final String PROP_OC_OVERRIDE_LOCATION = "ocOverrideLocation";

public void setOcOverride(boolean value) {
Connection connection = getConnection();
if(connection != null) {
Map<String, Object> properties = connection.getExtendedProperties();
Map<String, Object> properties = getExtendedProperties();
if( properties != null ) {
Object old = properties.get(ICommonAttributes.OC_OVERRIDE_KEY);
connection.setExtendedProperty(ICommonAttributes.OC_OVERRIDE_KEY, value);
properties.put(ICommonAttributes.OC_OVERRIDE_KEY, value);
firePropertyChange(PROP_OC_OVERRIDE, old, value);
}
}

public boolean getOcOverride() {
Connection connection = getConnection();
if(connection != null) {
Map<String, Object> properties = connection.getExtendedProperties();
Map<String, Object> properties = getExtendedProperties();
if( properties != null ) {
return ((Boolean)(ObjectUtils.defaultIfNull(properties.get(ICommonAttributes.OC_OVERRIDE_KEY), false))).booleanValue();
}
return false;
}

public void setOcOverrideLocation(String value) {
Connection connection = getConnection();
if(connection != null) {
Map<String, Object> properties = connection.getExtendedProperties();
Map<String, Object> properties = getExtendedProperties();
if( properties != null ) {
Object old = properties.get(ICommonAttributes.OC_LOCATION_KEY);
connection.setExtendedProperty(ICommonAttributes.OC_LOCATION_KEY, value);
properties.put(ICommonAttributes.OC_LOCATION_KEY, value);
firePropertyChange(PROP_OC_OVERRIDE_LOCATION, old, value);
}
}

public String getOcOverrideLocation() {
if( getOcOverride()) {
Connection connection = getConnection();
if(connection != null) {
Map<String, Object> properties = connection.getExtendedProperties();
Map<String, Object> properties = getExtendedProperties();
if( properties != null ) {
return (String) ObjectUtils.defaultIfNull(properties.get(ICommonAttributes.OC_LOCATION_KEY), "");
}
}
return OpenShiftCorePreferences.INSTANCE.getOCBinaryLocation();
}

public void setRegistryURL(String value) {
Connection connection = getConnection();
if(connection != null) {
Map<String, Object> properties = connection.getExtendedProperties();
Map<String, Object> properties = getExtendedProperties();
if( properties != null ) {
Object old = properties.get(ICommonAttributes.IMAGE_REGISTRY_URL_KEY);
connection.setExtendedProperty(ICommonAttributes.IMAGE_REGISTRY_URL_KEY, value);
properties.put(ICommonAttributes.IMAGE_REGISTRY_URL_KEY, value);
firePropertyChange(PROP_REGISTRY_URL, old, value);
}
}

public String getRegistryURL() {
Connection connection = getConnection();
if(connection != null) {
Map<String, Object> properties = connection.getExtendedProperties();
Map<String, Object> properties = getExtendedProperties();
if( properties != null ) {
return (String) ObjectUtils.defaultIfNull(properties.get(ICommonAttributes.IMAGE_REGISTRY_URL_KEY), "");
}
return "";
}

public void setClusterNamespace(String value) {
Connection connection = getConnection();
if(connection != null) {
Map<String, Object> properties = connection.getExtendedProperties();
Map<String, Object> properties = getExtendedProperties();
if( properties != null ) {
Object old = properties.get(ICommonAttributes.CLUSTER_NAMESPACE_KEY);
connection.setExtendedProperty(ICommonAttributes.CLUSTER_NAMESPACE_KEY, value);
properties.put(ICommonAttributes.CLUSTER_NAMESPACE_KEY, value);
firePropertyChange(PROP_CLUSTER_NAMESPACE, old, value);
}
}

public String getClusterNamespace() {
Connection connection = getConnection();
if(connection != null) {
return connection.getClusterNamespace();
}
Map<String, Object> properties = getExtendedProperties();
if( properties != null ) {
return (String)properties.getOrDefault(ICommonAttributes.CLUSTER_NAMESPACE_KEY, ICommonAttributes.COMMON_NAMESPACE);
}
return ICommonAttributes.COMMON_NAMESPACE;
}


}

private class ConnectionAdvancedPropertiesProvider implements IConnectionAdvancedPropertiesProvider {
Expand All @@ -456,4 +461,12 @@ public void run() {
}

}

@Override
public void saveChanges(ConnectionWizardPageModel pageModel) {
IConnection c = pageModel.getConnection();
if( c instanceof Connection && getExtendedProperties() != null ) {
((Connection)c).setExtendedProperties(getExtendedProperties());
}
}
}