Fixed review findings

This commit is contained in:
Nonnenmacher Fabian (INST-ICM/BSV-AS)
2016-03-04 12:02:59 +01:00
parent 0e9991b0a4
commit 9646e1eabc
17 changed files with 152 additions and 197 deletions

View File

@@ -75,8 +75,7 @@ public abstract class SpServerRtException extends RuntimeException {
}
/**
*
* @return the SpServerError
* @return the SpServerError which is wrapped by this exception
*/
public SpServerError getError() {
return error;

View File

@@ -9,7 +9,7 @@
package org.eclipse.hawkbit.tenancy.configuration;
import java.util.Arrays;
import java.util.NoSuchElementException;
import java.util.Optional;
import org.eclipse.hawkbit.tenancy.configuration.validator.TenantConfigurationBooleanValidator;
import org.eclipse.hawkbit.tenancy.configuration.validator.TenantConfigurationPollingDurationValidator;
@@ -84,10 +84,18 @@ public enum TenantConfigurationKey {
private final Class<? extends TenantConfigurationValidator> validator;
/**
*
* @param key
* the property key name
* @param allowedValues
* @param defaultKeyName
* the allowed values for this specific key
* @param dataType
* the class of the property
* @param defaultValue
* value which should be returned, in case there is no value in
* the database
* @param validator
* Validator which validates, that property is of correct format
*/
private TenantConfigurationKey(final String key, final String defaultKeyName, final Class<?> dataType,
final String defaultValue, final Class<? extends TenantConfigurationValidator> validator) {
@@ -96,7 +104,6 @@ public enum TenantConfigurationKey {
this.defaultKeyName = defaultKeyName;
this.defaultValue = defaultValue;
this.validator = validator;
}
/**
@@ -138,17 +145,14 @@ public enum TenantConfigurationKey {
* @param value
* which will be validated
* @throws TenantConfigurationValidatorException
* is thrown when object is invalid.
* is thrown, when object is invalid
*/
public void validate(final ApplicationContext context, final Object value)
throws TenantConfigurationValidatorException {
final TenantConfigurationValidator createBean = context.getAutowireCapableBeanFactory().createBean(validator);
public void validate(final ApplicationContext context, final Object value) {
final TenantConfigurationValidator createdBean = context.getAutowireCapableBeanFactory().createBean(validator);
try {
createBean.validate(value);
} catch (final TenantConfigurationValidatorException ex) {
throw ex;
createdBean.validate(value);
} finally {
context.getAutowireCapableBeanFactory().destroyBean(createBean);
context.getAutowireCapableBeanFactory().destroyBean(createdBean);
}
}
@@ -156,17 +160,15 @@ public enum TenantConfigurationKey {
* @param keyName
* name of the TenantConfigurationKey
* @return the TenantConfigurationKey with the name keyName
* @throws InvalidTenantConfigurationKeyException
* if there is no TenantConfigurationKey with the name keyName
*/
public static TenantConfigurationKey fromKeyName(final String keyName)
throws InvalidTenantConfigurationKeyException {
public static TenantConfigurationKey fromKeyName(final String keyName) {
try {
return Arrays.stream(TenantConfigurationKey.values()).filter(conf -> conf.getKeyName().equals(keyName))
.findFirst().get();
} catch (final NoSuchElementException e) {
throw new InvalidTenantConfigurationKeyException("The given configuration key name does not exist.");
final Optional<TenantConfigurationKey> optKey = Arrays.stream(TenantConfigurationKey.values())
.filter(conf -> conf.getKeyName().equals(keyName)).findFirst();
if (optKey.isPresent()) {
return optKey.get();
}
throw new InvalidTenantConfigurationKeyException("The given configuration key name does not exist.");
}
}

View File

@@ -16,13 +16,12 @@ import org.eclipse.hawkbit.tenancy.configuration.DurationHelper;
import org.springframework.beans.factory.annotation.Autowired;
/**
* specific tenant configuration validator, which validates that the given value
* is a String in the correct duration format..
* This class is used to validate, that the property is a String and that it is
* in the correct duration format.
*
*/
public class TenantConfigurationPollingDurationValidator implements TenantConfigurationValidator {
// private final ControllerPollProperties properties;
private final DurationHelper durationHelper = new DurationHelper();
private final Duration minDuration;
@@ -35,8 +34,6 @@ public class TenantConfigurationPollingDurationValidator implements TenantConfig
*/
@Autowired
public TenantConfigurationPollingDurationValidator(final ControllerPollProperties properties) {
// this.properties = properties;
minDuration = durationHelper.formattedStringToDuration(properties.getMinPollingTime());
maxDuration = durationHelper.formattedStringToDuration(properties.getMaxPollingTime());
}

View File

@@ -35,7 +35,7 @@ public class HttpControllerPreAuthenticateSecurityTokenFilter extends AbstractHt
/**
* Constructor.
*
* @param systemManagement
* @param tenantConfigurationManagement
* the system management service to retrieve configuration
* properties
* @param tenantAware
@@ -44,6 +44,8 @@ public class HttpControllerPreAuthenticateSecurityTokenFilter extends AbstractHt
* @param controllerManagement
* the controller management to retrieve the specific target
* security token to verify
* @param systemSecurityContext
* the system security context
*/
public HttpControllerPreAuthenticateSecurityTokenFilter(
final TenantConfigurationManagement tenantConfigurationManagement, final TenantAware tenantAware,

View File

@@ -27,12 +27,17 @@ public class HttpControllerPreAuthenticatedGatewaySecurityTokenFilter
/**
* Constructor.
*
* @param tenantConfigurationManagement
* the system management service to retrieve configuration
* properties
* @param systemManagement
* the system management service to retrieve configuration
* properties
* @param tenantAware
* the tenant aware service to get configuration for the specific
* tenant
* @param systemSecurityContext
* * @param systemSecurityContext the system security context
*/
public HttpControllerPreAuthenticatedGatewaySecurityTokenFilter(
final TenantConfigurationManagement tenantConfigurationManagement, final TenantAware tenantAware,

View File

@@ -35,13 +35,15 @@ public class HttpControllerPreAuthenticatedSecurityHeaderFilter extends Abstract
* @param caAuthorityNameHeader
* the http-header which holds the ca-authority name of the
* certificate
* @param systemManagement
* the system management service to retrieve configuration
* properties to check if the header authentication is enabled
* for this tenant
* @param tenantConfigurationManagement
* the tenant configuration management service to retrieve
* configuration properties to check if the header authentication
* is enabled for this tenant
* @param tenantAware
* the tenant aware service to get configuration for the specific
* tenant
* @param systemSecurityContext
* the system security context
*/
public HttpControllerPreAuthenticatedSecurityHeaderFilter(final String caCommonNameHeader,
final String caAuthorityNameHeader, final TenantConfigurationManagement tenantConfigurationManagement,

View File

@@ -84,7 +84,7 @@ public class TenantConfigurationManagement implements EnvironmentAware {
@PreAuthorize(value = SpringEvalExpressions.HAS_AUTH_TENANT_CONFIGURATION + SpringEvalExpressions.HAS_AUTH_OR
+ SpringEvalExpressions.IS_SYSTEM_CODE)
public <T> TenantConfigurationValue<T> getConfigurationValue(final TenantConfigurationKey configurationKey,
final Class<T> propertyType) throws TenantConfigurationValidatorException {
final Class<T> propertyType) {
if (!configurationKey.getDataType().isAssignableFrom(propertyType)) {
throw new TenantConfigurationValidatorException(
@@ -130,8 +130,7 @@ public class TenantConfigurationManagement implements EnvironmentAware {
* {@code propertyType}
*/
@PreAuthorize(value = SpringEvalExpressions.HAS_AUTH_TENANT_CONFIGURATION)
public TenantConfigurationValue<?> getConfigurationValue(final TenantConfigurationKey configurationKey)
throws TenantConfigurationValidatorException {
public TenantConfigurationValue<?> getConfigurationValue(final TenantConfigurationKey configurationKey) {
return getConfigurationValue(configurationKey, configurationKey.getDataType());
}
@@ -156,8 +155,8 @@ public class TenantConfigurationManagement implements EnvironmentAware {
* {@code propertyType}
*/
@PreAuthorize(value = SpringEvalExpressions.HAS_AUTH_TENANT_CONFIGURATION)
public <T> T getGlobalConfigurationValue(final TenantConfigurationKey configurationKey, final Class<T> propertyType)
throws TenantConfigurationValidatorException {
public <T> T getGlobalConfigurationValue(final TenantConfigurationKey configurationKey,
final Class<T> propertyType) {
if (!configurationKey.getDataType().isAssignableFrom(propertyType)) {
throw new TenantConfigurationValidatorException(
@@ -167,10 +166,11 @@ public class TenantConfigurationManagement implements EnvironmentAware {
final T valueInProperties = environment.getProperty(configurationKey.getDefaultKeyName(), propertyType);
if (valueInProperties != null) {
return valueInProperties;
if (valueInProperties == null) {
return conversionService.convert(configurationKey.getDefaultValue(), propertyType);
}
return conversionService.convert(configurationKey.getDefaultValue(), propertyType);
return valueInProperties;
}
/**
@@ -206,14 +206,11 @@ public class TenantConfigurationManagement implements EnvironmentAware {
TenantConfiguration tenantConfiguration = tenantConfigurationRepository
.findByKey(configurationKey.getKeyName());
if (tenantConfiguration != null)
{
tenantConfiguration.setValue(value.toString());
} else
{
if (tenantConfiguration == null) {
tenantConfiguration = new TenantConfiguration(configurationKey.getKeyName(), value.toString());
} else {
tenantConfiguration.setValue(value.toString());
}
final TenantConfiguration updatedTenantConfiguration = tenantConfigurationRepository.save(tenantConfiguration);

View File

@@ -320,22 +320,21 @@ public class TargetInfo implements Persistable<Long>, Serializable {
* before this method returns {@code null}
*/
public PollStatus getPollStatus() {
if (lastTargetQuery != null) {
final Duration pollTime = durationHelper.formattedStringToDuration(TenantConfigurationManagement
.getInstance().getConfigurationValue(TenantConfigurationKey.POLLING_TIME_INTERVAL, String.class)
.getValue());
final Duration overdueTime = durationHelper
.formattedStringToDuration(TenantConfigurationManagement.getInstance()
.getConfigurationValue(TenantConfigurationKey.POLLING_OVERDUE_TIME_INTERVAL, String.class)
.getValue());
final LocalDateTime currentDate = LocalDateTime.now();
final LocalDateTime lastPollDate = LocalDateTime.ofInstant(Instant.ofEpochMilli(lastTargetQuery),
ZoneId.systemDefault());
final LocalDateTime nextPollDate = lastPollDate.plus(pollTime);
final LocalDateTime overdueDate = nextPollDate.plus(overdueTime);
return new PollStatus(lastPollDate, nextPollDate, overdueDate, currentDate);
if (lastTargetQuery == null) {
return null;
}
return null;
final Duration pollTime = durationHelper.formattedStringToDuration(TenantConfigurationManagement.getInstance()
.getConfigurationValue(TenantConfigurationKey.POLLING_TIME_INTERVAL, String.class).getValue());
final Duration overdueTime = durationHelper.formattedStringToDuration(TenantConfigurationManagement
.getInstance().getConfigurationValue(TenantConfigurationKey.POLLING_OVERDUE_TIME_INTERVAL, String.class)
.getValue());
final LocalDateTime currentDate = LocalDateTime.now();
final LocalDateTime lastPollDate = LocalDateTime.ofInstant(Instant.ofEpochMilli(lastTargetQuery),
ZoneId.systemDefault());
final LocalDateTime nextPollDate = lastPollDate.plus(pollTime);
final LocalDateTime overdueDate = nextPollDate.plus(overdueTime);
return new PollStatus(lastPollDate, nextPollDate, overdueDate, currentDate);
}
/**
@@ -378,23 +377,14 @@ public class TargetInfo implements Persistable<Long>, Serializable {
return lastPollDate;
}
/**
* @return the nextPollDate
*/
public LocalDateTime getNextPollDate() {
return nextPollDate;
}
/**
* @return the overdueDate
*/
public LocalDateTime getOverdueDate() {
return overdueDate;
}
/**
* @return the current date
*/
public LocalDateTime getCurrentDate() {
return currentDate;
}

View File

@@ -16,16 +16,16 @@ package org.eclipse.hawkbit.repository.model;
*/
public class TenantConfigurationValue<T> {
private T value;
private Long lastModifiedAt;
private String lastModifiedBy;
private Long createdAt;
private String createdBy;
private boolean isGlobal = true;
private TenantConfigurationValue() {
}
private T value = null;
private boolean isGlobal = true;
private Long lastModifiedAt = null;
private String lastModifiedBy = null;
private Long createdAt = null;
private String createdBy = null;
/**
* Gets the value.
*

View File

@@ -15,20 +15,25 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonInclude.Include;
/**
* A json annotated rest model for a tenant configuration value to RESTful API
* representation.
*
*/
@JsonInclude(Include.NON_NULL)
@JsonIgnoreProperties(ignoreUnknown = true)
public class TenantConfigurationValueRest extends ResourceSupport {
@JsonInclude(Include.ALWAYS)
private Object value = null;
private Object value;
@JsonInclude(Include.ALWAYS)
private boolean isGlobal = true;
private Long lastModifiedAt = null;
private String lastModifiedBy = null;
private Long createdAt = null;
private String createdBy = null;
private Long lastModifiedAt;
private String lastModifiedBy;
private Long createdAt;
private String createdBy;
public Object getValue() {
return value;

View File

@@ -39,14 +39,8 @@ import com.vaadin.ui.VerticalLayout;
public class AuthenticationConfigurationView extends BaseConfigurationView
implements ConfigurationGroup, ConfigurationItem.ConfigurationItemChangeListener, ValueChangeListener {
/**
*
*/
private static final String DIST_CHECKBOX_STYLE = "dist-checkbox-style";
/**
*
*/
private static final long serialVersionUID = 1L;
@Autowired
@@ -119,13 +113,6 @@ public class AuthenticationConfigurationView extends BaseConfigurationView
setCompositionRoot(rootPanel);
}
/*
* (non-Javadoc)
*
* @see
* org.eclipse.hawkbit.server.ui.tenantconfiguration.ConfigurationGroup#save
* ()
*/
@Override
public void save() {
certificateAuthenticationConfigurationItem.save();
@@ -133,13 +120,6 @@ public class AuthenticationConfigurationView extends BaseConfigurationView
gatewaySecurityTokenAuthenticationConfigurationItem.save();
}
/*
* (non-Javadoc)
*
* @see
* org.eclipse.hawkbit.server.ui.tenantconfiguration.ConfigurationGroup#undo
* ()
*/
@Override
public void undo() {
certificateAuthenticationConfigurationItem.undo();
@@ -155,37 +135,32 @@ public class AuthenticationConfigurationView extends BaseConfigurationView
notifyConfigurationChanged();
}
/*
* (non-Javadoc)
*
* @see com.vaadin.data.Property.ValueChangeListener#valueChange(com.vaadin.
* data.Property. ValueChangeEvent)
*/
@Override
public void valueChange(final ValueChangeEvent event) {
if (event.getProperty() instanceof CheckBox) {
notifyConfigurationChanged();
if (!(event.getProperty() instanceof CheckBox)) {
return;
}
CheckBox checkBox = (CheckBox) event.getProperty();
AuthenticationConfigurationItem configurationItem = null;
notifyConfigurationChanged();
if (checkBox == gatewaySecTokenCheckBox) {
configurationItem = gatewaySecurityTokenAuthenticationConfigurationItem;
} else if (checkBox == targetSecTokenCheckBox) {
configurationItem = targetSecurityTokenAuthenticationConfigurationItem;
} else if (checkBox == certificateAuthCheckbox) {
configurationItem = certificateAuthenticationConfigurationItem;
} else {
return;
}
final CheckBox checkBox = (CheckBox) event.getProperty();
AuthenticationConfigurationItem configurationItem;
if (checkBox.getValue()) {
configurationItem.configEnable();
} else {
configurationItem.configDisable();
}
if (checkBox == gatewaySecTokenCheckBox) {
configurationItem = gatewaySecurityTokenAuthenticationConfigurationItem;
} else if (checkBox == targetSecTokenCheckBox) {
configurationItem = targetSecurityTokenAuthenticationConfigurationItem;
} else if (checkBox == certificateAuthCheckbox) {
configurationItem = certificateAuthenticationConfigurationItem;
} else {
return;
}
if (checkBox.getValue()) {
configurationItem.configEnable();
} else {
configurationItem.configDisable();
}
}
}

View File

@@ -10,6 +10,9 @@ package org.eclipse.hawkbit.ui.tenantconfiguration;
import java.io.Serializable;
/**
* represents an configurationItem, which can be modified by the user
*/
public interface ConfigurationItem {
/**
@@ -41,4 +44,4 @@ public interface ConfigurationItem {
*/
void configurationHasChanged();
}
}
}

View File

@@ -44,8 +44,7 @@ import com.vaadin.ui.VerticalLayout;
*/
@SpringView(name = TenantConfigurationDashboardView.VIEW_NAME, ui = HawkbitUI.class)
@ViewScope
public class TenantConfigurationDashboardView extends CustomComponent
implements View, ConfigurationItemChangeListener {
public class TenantConfigurationDashboardView extends CustomComponent implements View, ConfigurationItemChangeListener {
public static final String VIEW_NAME = "spSystemConfig";
private static final long serialVersionUID = 1L;
@@ -68,7 +67,7 @@ public class TenantConfigurationDashboardView extends CustomComponent
private Button saveConfigurationBtn;
private Button undoConfigurationBtn;
private List<ConfigurationGroup> configurationViews = new ArrayList<ConfigurationGroup>();
private final List<ConfigurationGroup> configurationViews = new ArrayList<ConfigurationGroup>();
/**
* init method adds all Configuration Views to the list of Views.
@@ -91,9 +90,7 @@ public class TenantConfigurationDashboardView extends CustomComponent
rootLayout.setMargin(true);
rootLayout.setSpacing(true);
configurationViews.forEach(view -> {
rootLayout.addComponent(view);
});
configurationViews.forEach(view -> rootLayout.addComponent(view));
final HorizontalLayout buttonContent = saveConfigurationButtonsLayout();
rootLayout.addComponent(buttonContent);
@@ -101,9 +98,7 @@ public class TenantConfigurationDashboardView extends CustomComponent
rootPanel.setContent(rootLayout);
setCompositionRoot(rootPanel);
configurationViews.forEach(view -> {
view.addChangeListener(this);
});
configurationViews.forEach(view -> view.addChangeListener(this));
}
private HorizontalLayout saveConfigurationButtonsLayout() {
@@ -132,23 +127,18 @@ public class TenantConfigurationDashboardView extends CustomComponent
private void saveConfiguration() {
boolean isUserInputValid = configurationViews.stream().allMatch(confView -> {
return confView.isUserInputValid();
});
final boolean isUserInputValid = configurationViews.stream().allMatch(confView -> confView.isUserInputValid());
if (isUserInputValid) {
configurationViews.forEach(confView -> {
confView.save();
});
// More methods
saveConfigurationBtn.setEnabled(false);
undoConfigurationBtn.setEnabled(false);
uINotification.displaySuccess(i18n.get("notification.configuration.save.successful"));
} else {
if (!isUserInputValid) {
uINotification.displayValidationError(i18n.get("notification.configuration.save.notpossible"));
return;
}
configurationViews.forEach(confView -> confView.save());
// More methods
saveConfigurationBtn.setEnabled(false);
undoConfigurationBtn.setEnabled(false);
uINotification.displaySuccess(i18n.get("notification.configuration.save.successful"));
}
private void undoConfiguration() {

View File

@@ -19,7 +19,7 @@ import org.eclipse.hawkbit.ui.utils.SPUILabelDefinitions;
import com.vaadin.ui.VerticalLayout;
/**
* Ab abstract authentication configuration item.
* abstract authentication configuration item.
*
*
*
@@ -28,9 +28,6 @@ import com.vaadin.ui.VerticalLayout;
abstract class AbstractAuthenticationTenantConfigurationItem extends VerticalLayout
implements AuthenticationConfigurationItem {
/**
*
*/
private static final long serialVersionUID = 1L;
private final TenantConfigurationKey configurationKey;
@@ -59,17 +56,9 @@ abstract class AbstractAuthenticationTenantConfigurationItem extends VerticalLay
addComponent(SPUIComponentProvider.getLabel(labelText, SPUILabelDefinitions.SP_LABEL_SIMPLE));
}
/*
* (non-Javadoc)
*
* @see org.eclipse.hawkbit.server.ui.tenantconfiguration.
* TenantConfigurationItem# isConfigEnabled()
*/
@Override
public boolean isConfigEnabled() {
final boolean b = tenantConfigurationManagement.getConfigurationValue(configurationKey, Boolean.class)
.getValue();
return b;
return tenantConfigurationManagement.getConfigurationValue(configurationKey, Boolean.class).getValue();
}
/**
@@ -90,14 +79,6 @@ abstract class AbstractAuthenticationTenantConfigurationItem extends VerticalLay
configurationChangeListeners.forEach(listener -> listener.configurationHasChanged());
}
/*
* (non-Javadoc)
*
* @see org.eclipse.hawkbit.server.ui.tenantconfiguration.
* TenantConfigurationItem# addConfigurationChangeListener
* (hawkbit.server.ui.tenantconfiguration.TenantConfigurationItem.
* TenantConfigurationChangeListener)
*/
@Override
public void addChangeListener(final ConfigurationItemChangeListener listener) {
configurationChangeListeners.add(listener);

View File

@@ -87,9 +87,10 @@ public class TargetSecurityTokenAuthenticationConfigurationItem extends Abstract
@Override
public void save() {
if (configurationEnabledChange) {
getTenantConfigurationManagement().addOrUpdateConfiguration(getConfigurationKey(), configurationEnabled);
if (!configurationEnabledChange) {
return;
}
getTenantConfigurationManagement().addOrUpdateConfiguration(getConfigurationKey(), configurationEnabled);
}
@Override

View File

@@ -56,14 +56,18 @@ public class DurationConfigField extends GridLayout implements ValueChangeListen
@Override
public void valueChange(final ValueChangeEvent event) {
if (event.getProperty() == checkBox) {
if (checkBox.getValue()) {
durationField.setEnabled(true);
} else {
durationField.setDuration(globalDuration);
durationField.setEnabled(false);
}
if (event.getProperty() != checkBox) {
return;
}
durationField.setEnabled(checkBox.getValue());
if (!checkBox.getValue()) {
durationField.setDuration(globalDuration);
}
durationField.setEnabled(false);
notifyConfigurationChanged();
}
@@ -102,11 +106,12 @@ public class DurationConfigField extends GridLayout implements ValueChangeListen
checkBox.setValue(false);
durationField.setDuration(globalDuration);
durationField.setEnabled(false);
} else {
checkBox.setValue(true);
durationField.setDuration(tenantDuration);
durationField.setEnabled(true);
return;
}
checkBox.setValue(true);
durationField.setDuration(tenantDuration);
durationField.setEnabled(true);
}
/**

View File

@@ -108,21 +108,22 @@ public class DurationField extends DateField {
// method. This method overwrites the super method, which is
// necessary, that parsing works correctly on pressing enter key
if (event.getProperty() instanceof DurationField) {
final Date value = (Date) event.getProperty().getValue();
if (!(event.getProperty() instanceof DurationField)) {
return;
}
final Date value = (Date) event.getProperty().getValue();
// setValue() calls valueChanged again, when the minimum is greater
// than the maximum this can lead to an endless loop
if (value != null && minimumDuration != null && maximumDuration != null
&& minimumDuration.before(maximumDuration)) {
// setValue() calls valueChanged again, when the minimum is greater
// than the maximum this can lead to an endless loop
if (value != null && minimumDuration != null && maximumDuration != null
&& minimumDuration.before(maximumDuration)) {
if (compareTimeOfDates(value, maximumDuration) > 0) {
((DateField) event.getProperty()).setValue(maximumDuration);
}
if (compareTimeOfDates(value, maximumDuration) > 0) {
((DateField) event.getProperty()).setValue(maximumDuration);
}
if (compareTimeOfDates(minimumDuration, value) > 0) {
((DateField) event.getProperty()).setValue(minimumDuration);
}
if (compareTimeOfDates(minimumDuration, value) > 0) {
((DateField) event.getProperty()).setValue(minimumDuration);
}
}
}