Skip to content

Commit

Permalink
Fix a bug where a null plugin setting throws an exception when attemp…
Browse files Browse the repository at this point in the history
…ting to validate that setting. Always return a non-null plugin configuration. (#1525)

Signed-off-by: David Venable <[email protected]>
  • Loading branch information
dlvenable authored Jun 17, 2022
1 parent 5cf985b commit 7d26f16
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

import javax.inject.Named;
import java.time.Duration;
import java.util.Collections;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -45,22 +47,22 @@ class PluginConfigurationConverter {
* Java Bean Validation 2.0.
*
* @param pluginConfigurationType the destination type
* @param pluginSetting The source {@link PluginSetting}
* @param pluginSetting The source {@link PluginSetting}
* @return The converted object of type pluginConfigurationType
* @throws InvalidPluginConfigurationException - If the plugin configuration is invalid
*/
public Object convert(final Class<?> pluginConfigurationType, final PluginSetting pluginSetting) {
Objects.requireNonNull(pluginConfigurationType);
Objects.requireNonNull(pluginSetting);

if(pluginConfigurationType.equals(PluginSetting.class))
if (pluginConfigurationType.equals(PluginSetting.class))
return pluginSetting;

final Object configuration = objectMapper.convertValue(pluginSetting.getSettings(), pluginConfigurationType);
final Object configuration = convertSettings(pluginConfigurationType, pluginSetting);

final Set<ConstraintViolation<Object>> constraintViolations = validator.validate(configuration);

if(!constraintViolations.isEmpty()) {
if (!constraintViolations.isEmpty()) {
final String violationsString = constraintViolations.stream()
.map(v -> v.getPropertyPath().toString() + " " + v.getMessage())
.collect(Collectors.joining(". "));
Expand All @@ -72,4 +74,11 @@ public Object convert(final Class<?> pluginConfigurationType, final PluginSettin

return configuration;
}

private Object convertSettings(final Class<?> pluginConfigurationType, final PluginSetting pluginSetting) {
Map<String, Object> settingsMap = pluginSetting.getSettings();
if (settingsMap == null)
settingsMap = Collections.emptyMap();
return objectMapper.convertValue(settingsMap, pluginConfigurationType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.CoreMatchers.sameInstance;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down Expand Up @@ -94,6 +95,21 @@ void convert_with_other_target_should_return_pluginSetting_object_directly() {
assertThat(convertedTestConfiguration.getMyValue(), equalTo(value));
}

@Test
void convert_with_other_target_should_return_empty_when_settings_are_null() {
given(pluginSetting.getSettings())
.willReturn(null);

final Object convertedConfiguration = createObjectUnderTest().convert(TestConfiguration.class, pluginSetting);

assertThat(convertedConfiguration, notNullValue());
assertThat(convertedConfiguration, instanceOf(TestConfiguration.class));

final TestConfiguration convertedTestConfiguration = (TestConfiguration) convertedConfiguration;

assertThat(convertedTestConfiguration.getMyValue(), nullValue());
}

@Test
void convert_with_other_target_should_validate_configuration() {

Expand Down

0 comments on commit 7d26f16

Please sign in to comment.