From 850f9e3bc992da77bdd73c08464f4dd0047f221f Mon Sep 17 00:00:00 2001 From: Andrea Ceccanti Date: Tue, 10 Nov 2020 18:35:59 +0100 Subject: [PATCH 1/2] Fix hr.properties parsing Fix buggy `membership_check.enabled` property parsing. Issue: https://issues.infn.it/jira/browse/VOMS-883 --- .../integration/cern/HrDbProperties.java | 3 +- .../integration/hr/HrDbConfiguratorTests.java | 30 +++++++++++++++---- .../integration/hr/HrDbPropertiesTest.java | 6 ++++ .../hr.properties} | 0 .../resources/cern/enabled-task/hr.properties | 8 +++++ 5 files changed, 40 insertions(+), 7 deletions(-) rename voms-admin-server/src/test/resources/cern/{config/orgdb.properties => disabled-task/hr.properties} (100%) create mode 100644 voms-admin-server/src/test/resources/cern/enabled-task/hr.properties diff --git a/voms-admin-server/src/main/java/org/glite/security/voms/admin/integration/cern/HrDbProperties.java b/voms-admin-server/src/main/java/org/glite/security/voms/admin/integration/cern/HrDbProperties.java index 91cdc645..0ca06762 100644 --- a/voms-admin-server/src/main/java/org/glite/security/voms/admin/integration/cern/HrDbProperties.java +++ b/voms-admin-server/src/main/java/org/glite/security/voms/admin/integration/cern/HrDbProperties.java @@ -187,7 +187,8 @@ public static final HrDbProperties fromProperties(Properties properties) { } if (properties.containsKey(MEMBERSHIP_CHECK_ENABLED_KEY)) { - config.getMembershipCheck().setEnabled(Boolean.parseBoolean(MEMBERSHIP_CHECK_ENABLED_KEY)); + config.getMembershipCheck() + .setEnabled(Boolean.parseBoolean(properties.getProperty(MEMBERSHIP_CHECK_ENABLED_KEY))); } config.getApi().setEndpoint(properties.getProperty(API_ENDPOINT_KEY)); diff --git a/voms-admin-server/src/test/java/integration/hr/HrDbConfiguratorTests.java b/voms-admin-server/src/test/java/integration/hr/HrDbConfiguratorTests.java index b4eccbe6..59543dc5 100644 --- a/voms-admin-server/src/test/java/integration/hr/HrDbConfiguratorTests.java +++ b/voms-admin-server/src/test/java/integration/hr/HrDbConfiguratorTests.java @@ -99,19 +99,37 @@ public void setup() { configurator.setUserDao(dao); configurator.setClock(CLOCK); - when(vomsConfig.getConfigurationDirectoryPath()).thenReturn("src/test/resources/cern/config"); - when(vomsConfig.getExternalValidatorProperty(eq("orgdb"), anyString(), anyString())) - .thenReturn("src/test/resources/cern/config/orgdb.properties"); + when(vomsConfig.getConfigurationDirectoryPath()) + .thenReturn("src/test/resources/cern/disabled-task"); + when(vomsConfig.getExternalValidatorProperty(eq("orgdb"), eq("configFile"), anyString())) + .thenReturn("src/test/resources/cern/disabled-task/hr.properties"); when(apiFactory.newHrDbApiService(any())).thenReturn(api); when(validatorFactory.newHrDbRequestValidator(any(), any())).thenReturn(validator); when(syncTaskFactory.buildSyncTask(any(), any(), any(), any())).thenReturn(syncTask); } @Test - public void testConfigurationFileIsFoundAndParsed() throws VOMSPluginConfigurationException { + public void testDisabledConfigurationFileIsFoundAndParsed() + throws VOMSPluginConfigurationException { + configurator.configure(); + verifyZeroInteractions(executorService);// no interactions as periodic sync is disabled + verifyZeroInteractions(syncTaskFactory);// no interactions as periodic sync is disabled + verify(manager).setRequestValidationContext(Mockito.eq(validator)); + } + + @Test + public void testEnabledConfigurationFileIsFoundAndParsed() + throws VOMSPluginConfigurationException { + when(vomsConfig.getConfigurationDirectoryPath()) + .thenReturn("src/test/resources/cern/enabled-task"); + when(vomsConfig.getExternalValidatorProperty(eq("orgdb"), eq("configFile"), anyString())) + .thenReturn("src/test/resources/cern/enabled-task/hr.properties"); configurator.configure(); - verifyZeroInteractions(executorService); - verifyZeroInteractions(syncTaskFactory); + + verify(executorService).scheduleAtFixedRate(isA(DatabaseTransactionTaskWrapper.class), + Mockito.anyLong(), Mockito.eq(TimeUnit.HOURS.toSeconds(12)), + Mockito.eq(TimeUnit.SECONDS)); + verify(manager).setRequestValidationContext(Mockito.eq(validator)); } diff --git a/voms-admin-server/src/test/java/integration/hr/HrDbPropertiesTest.java b/voms-admin-server/src/test/java/integration/hr/HrDbPropertiesTest.java index 53fca7f3..3ce72f94 100644 --- a/voms-admin-server/src/test/java/integration/hr/HrDbPropertiesTest.java +++ b/voms-admin-server/src/test/java/integration/hr/HrDbPropertiesTest.java @@ -71,6 +71,12 @@ public void testValueParsing() { assertThat(hrConfig.getMembershipCheck().getPeriodInSeconds(), is(86400L)); } + + @Test + public void testFileParsing() { + + } + @Test public void testApiTimeoutChecks() { Properties props = new Properties(); diff --git a/voms-admin-server/src/test/resources/cern/config/orgdb.properties b/voms-admin-server/src/test/resources/cern/disabled-task/hr.properties similarity index 100% rename from voms-admin-server/src/test/resources/cern/config/orgdb.properties rename to voms-admin-server/src/test/resources/cern/disabled-task/hr.properties diff --git a/voms-admin-server/src/test/resources/cern/enabled-task/hr.properties b/voms-admin-server/src/test/resources/cern/enabled-task/hr.properties new file mode 100644 index 00000000..6556749f --- /dev/null +++ b/voms-admin-server/src/test/resources/cern/enabled-task/hr.properties @@ -0,0 +1,8 @@ + experiment=cms + membership_check.enabled=true + membership_check.start_hour=4 + membership_check.run_at_startup=false + api.endpoint=http://localhost:8080 + api.username=user + api.password=password + api.timeout_secs=2 \ No newline at end of file From 5fa1ad5d3e341b79df37211a130994a9e1aa091f Mon Sep 17 00:00:00 2001 From: Andrea Ceccanti Date: Tue, 10 Nov 2020 18:45:03 +0100 Subject: [PATCH 2/2] More HR properties parsing tests --- .../integration/hr/HrDbPropertiesTest.java | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/voms-admin-server/src/test/java/integration/hr/HrDbPropertiesTest.java b/voms-admin-server/src/test/java/integration/hr/HrDbPropertiesTest.java index 3ce72f94..c8facff9 100644 --- a/voms-admin-server/src/test/java/integration/hr/HrDbPropertiesTest.java +++ b/voms-admin-server/src/test/java/integration/hr/HrDbPropertiesTest.java @@ -17,9 +17,13 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.fail; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; import java.util.Properties; import org.glite.security.voms.admin.integration.cern.HrDbError; @@ -73,7 +77,24 @@ public void testValueParsing() { @Test - public void testFileParsing() { + public void testFileParsing() throws FileNotFoundException, IOException { + + Properties props = new Properties(); + props + .load(new FileInputStream(new File("src/test/resources/cern/enabled-task/hr.properties"))); + + HrDbProperties hrDbProps = HrDbProperties.fromProperties(props); + + assertThat(hrDbProps.getExperimentName(), is("cms")); + assertThat(hrDbProps.getMembershipCheck().isEnabled(), is(true)); + assertThat(hrDbProps.getMembershipCheck().getStartHour(), is(4)); + assertThat(hrDbProps.getMembershipCheck().isRunAtStartup(), is(false)); + assertThat(hrDbProps.getApi().getEndpoint(), is("http://localhost:8080")); + assertThat(hrDbProps.getApi().getUsername(), is("user")); + assertThat(hrDbProps.getApi().getPassword(), is("password")); + assertThat(hrDbProps.getApi().getTimeoutInSeconds(), is(2L)); + + }