From 31c371501ae161dad9776bbf5d3b48af28ed5b01 Mon Sep 17 00:00:00 2001 From: Glenn Renfro Date: Fri, 1 Nov 2024 08:49:58 -0400 Subject: [PATCH 1/2] Remove schedule. deprecated property key prefix Use deployer. property prefix when setting schedule properties Originally scheduled properties uses schedule. as a prefix for all the settings. However it was determined they were just a subset of deployer properties and thus could be deprecated Signed-off-by: Glenn Renfro polish Signed-off-by: Glenn Renfro --- .../service/impl/DefaultSchedulerService.java | 21 +++++++------------ .../TaskSchedulerControllerTests.java | 8 +++---- ...ultSchedulerServiceMultiplatformTests.java | 11 +++++++--- .../impl/DefaultSchedulerServiceTests.java | 17 +++++++++------ whats-new.adoc | 3 ++- 5 files changed, 32 insertions(+), 28 deletions(-) diff --git a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerService.java b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerService.java index 2c3d94dcec..a8ab2e09b2 100644 --- a/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerService.java +++ b/spring-cloud-dataflow-server-core/src/main/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerService.java @@ -283,7 +283,7 @@ public void schedule( AppDefinition revisedDefinition = TaskServiceUtils.mergeAndExpandAppProperties(taskDefinition, metadataResource, appDeploymentProperties, visibleProperties); DeploymentPropertiesUtils.validateDeploymentProperties(taskDeploymentProperties); - taskDeploymentProperties = extractAndQualifySchedulerProperties(taskDeploymentProperties); + taskDeploymentProperties = filterPrefixedProperties(taskDeploymentProperties); deployerDeploymentProperties.putAll(taskDeploymentProperties); scheduleName = validateScheduleNameForPlatform(launcher.getType(), scheduleName); ScheduleRequest scheduleRequest = new ScheduleRequest(revisedDefinition, @@ -501,22 +501,15 @@ private List limitScheduleInfoResultSize( } /** - * Retain only properties that are meant for the scheduler of a given task(those - * that start with {@code scheduler.}and qualify all - * property values with the {@code spring.cloud.scheduler.} prefix. + * Provided a filtered Map that removes entries prefixed with "app." or "deployer.". * - * @param input the scheduler properties - * @return scheduler properties for the task + * @param input the properties + * @return deployer properties for the schedule */ - @Deprecated - private static Map extractAndQualifySchedulerProperties(Map input) { - final String prefix = "scheduler."; - final int prefixLength = prefix.length(); - + private static Map filterPrefixedProperties(Map input) { return new TreeMap<>(input).entrySet().stream() - .filter(kv -> kv.getKey().startsWith(prefix)) - .collect(Collectors.toMap(kv -> "spring.cloud.deployer." + kv.getKey().substring(prefixLength), Map.Entry::getValue, - (fromWildcard, fromApp) -> fromApp)); + .filter(kv -> (!kv.getKey().startsWith("deployer.") && !kv.getKey().startsWith("app."))) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); } protected Resource getTaskResource(String taskDefinitionName, String version) { diff --git a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/TaskSchedulerControllerTests.java b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/TaskSchedulerControllerTests.java index c7d75def65..dd0a4daf03 100644 --- a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/TaskSchedulerControllerTests.java +++ b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/controller/TaskSchedulerControllerTests.java @@ -189,7 +189,7 @@ private void createAndVerifySchedule(String scheduleName, String createdSchedule repository.save(new TaskDefinition("testDefinition", "testApp")); mockMvc.perform(post("/tasks/schedules").param("taskDefinitionName", "testDefinition") - .param("scheduleName", scheduleName).param("properties", "scheduler.cron.expression=* * * * *") + .param("scheduleName", scheduleName).param("properties", "deployer.testApp.cron.expression=* * * * *") .accept(MediaType.APPLICATION_JSON)).andDo(print()).andExpect(status().isCreated()); assertThat(simpleTestScheduler.list()).hasSize(1); ScheduleInfo scheduleInfo = simpleTestScheduler.list().get(0); @@ -253,7 +253,7 @@ private String createScheduleWithArguments(String arguments) throws Exception { mockMvc.perform(post("/tasks/schedules").param("taskDefinitionName", "testDefinition") .param("scheduleName", "mySchedule") .param("properties", - "scheduler.cron.expression=* * * * *,app.testApp.prop1=foo,app.testApp.prop2.secret=kenny,deployer.*.prop1.secret=cartman,deployer.*.prop2.password=kyle") + "deployer.testApp.cron.expression=* * * * *,app.testApp.prop1=foo,app.testApp.prop2.secret=kenny,deployer.*.prop1.secret=cartman,deployer.*.prop2.password=kyle") .param("arguments", arguments) .accept(MediaType.APPLICATION_JSON)).andDo(print()).andExpect(status().isCreated()); assertThat(simpleTestScheduler.list()).hasSize(1); @@ -283,7 +283,7 @@ void createScheduleBadCron() throws Exception { mockMvc.perform(post("/tasks/schedules").param("taskDefinitionName", "testDefinition") .param("scheduleName", "myScheduleBadCron") .param("properties", - "scheduler.cron.expression=" + SimpleTestScheduler.INVALID_CRON_EXPRESSION) + "deployer.testApp.cron.expression=" + SimpleTestScheduler.INVALID_CRON_EXPRESSION) .accept(MediaType.APPLICATION_JSON)).andExpect(status().isBadRequest()); } @@ -339,7 +339,7 @@ private void createSampleSchedule(String scheduleName) { private void createSampleSchedule(String taskDefinitionName, String scheduleName) { Map properties = new HashMap<>(); - properties.put("scheduler.testApp." + SchedulerPropertyKeys.CRON_EXPRESSION, "* * * * *"); + properties.put("deployer.testApp." + SchedulerPropertyKeys.CRON_EXPRESSION, "* * * * *"); schedulerService.schedule(scheduleName, taskDefinitionName, properties, new ArrayList<>(), null); } diff --git a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerServiceMultiplatformTests.java b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerServiceMultiplatformTests.java index 7092a08d59..2c34032cfc 100644 --- a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerServiceMultiplatformTests.java +++ b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerServiceMultiplatformTests.java @@ -93,7 +93,7 @@ @AutoConfigureTestDatabase(replace = Replace.ANY) public class DefaultSchedulerServiceMultiplatformTests { - private static final String DATA_FLOW_SCHEDULER_PREFIX = "scheduler."; + private static final String DATA_FLOW_DEPLOYER_PREFIX = "deployer.demo."; private static final String DEPLOYER_PREFIX = "spring.cloud.deployer."; @@ -166,8 +166,8 @@ void setup() throws Exception { initializeSuccessfulRegistry(); this.testProperties = new HashMap<>(); - this.testProperties.put(DATA_FLOW_SCHEDULER_PREFIX + "AAAA", "* * * * *"); - this.testProperties.put(DATA_FLOW_SCHEDULER_PREFIX + "EXPRESSION", "* * * * *"); + this.testProperties.put(DATA_FLOW_DEPLOYER_PREFIX + "AAAA", "* * * * *"); + this.testProperties.put(DATA_FLOW_DEPLOYER_PREFIX + "EXPRESSION", "* * * * *"); this.testProperties.put("version." + BASE_DEFINITION_NAME, "1.0.0"); this.resolvedProperties = new HashMap<>(); this.resolvedProperties.put(DEPLOYER_PREFIX + "AAAA", "* * * * *"); @@ -243,6 +243,11 @@ public void testScheduleWithLongName() { @Test void scheduleCTR() { + this.testProperties = new HashMap<>(); + this.testProperties.put(DATA_FLOW_DEPLOYER_PREFIX + "AAAA", "* * * * *"); + this.testProperties.put(DATA_FLOW_DEPLOYER_PREFIX + "EXPRESSION", "* * * * *"); + this.testProperties.put("deployer.composed-task-runner." + "AAAA", "* * * * *"); + this.testProperties.put("deployer.composed-task-runner." + "EXPRESSION", "* * * * *"); schedulerService.schedule(BASE_SCHEDULE_NAME, CTR_DEFINITION_NAME, this.testProperties, this.commandLineArgs, KUBERNETES_PLATFORM); verifyScheduleExistsInScheduler(createScheduleInfo(BASE_SCHEDULE_NAME, CTR_DEFINITION_NAME)); } diff --git a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerServiceTests.java b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerServiceTests.java index d534d431e1..25dc20096b 100644 --- a/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerServiceTests.java +++ b/spring-cloud-dataflow-server-core/src/test/java/org/springframework/cloud/dataflow/server/service/impl/DefaultSchedulerServiceTests.java @@ -94,9 +94,9 @@ @AutoConfigureTestDatabase(replace = Replace.ANY) public class DefaultSchedulerServiceTests { - private static final String DATA_FLOW_SCHEDULER_PREFIX = "scheduler."; + private static final String DATA_FLOW_DEPLOYER_PREFIX = "deployer.demo."; - private static final String SCHEDULER_PREFIX = "spring.cloud.deployer."; + private static final String DEPLOYER_PREFIX = "spring.cloud.deployer."; private static final String BASE_SCHEDULE_NAME = "myTaskSchedule"; @@ -161,11 +161,11 @@ void setup() throws Exception{ initializeSuccessfulRegistry(); this.testProperties = new HashMap<>(); - this.testProperties.put(DATA_FLOW_SCHEDULER_PREFIX + "AAAA", "* * * * *"); - this.testProperties.put(DATA_FLOW_SCHEDULER_PREFIX + "EXPRESSION", "* * * * *"); + this.testProperties.put(DATA_FLOW_DEPLOYER_PREFIX + "AAAA", "* * * * *"); + this.testProperties.put(DATA_FLOW_DEPLOYER_PREFIX + "EXPRESSION", "* * * * *"); this.resolvedProperties = new HashMap<>(); - this.resolvedProperties.put(SCHEDULER_PREFIX + "AAAA", "* * * * *"); - this.resolvedProperties.put(SCHEDULER_PREFIX + "EXPRESSION", "* * * * *"); + this.resolvedProperties.put(DEPLOYER_PREFIX + "AAAA", "* * * * *"); + this.resolvedProperties.put(DEPLOYER_PREFIX + "EXPRESSION", "* * * * *"); this.commandLineArgs = new ArrayList<>(); } @@ -241,6 +241,11 @@ public void testScheduleWithLongName(){ @Test void scheduleCTR(){ + this.testProperties = new HashMap<>(); + this.testProperties.put(DATA_FLOW_DEPLOYER_PREFIX + "AAAA", "* * * * *"); + this.testProperties.put(DATA_FLOW_DEPLOYER_PREFIX + "EXPRESSION", "* * * * *"); + this.testProperties.put("deployer.composed-task-runner." + "AAAA", "* * * * *"); + this.testProperties.put("deployer.composed-task-runner." + "EXPRESSION", "* * * * *"); schedulerService.schedule(BASE_SCHEDULE_NAME, CTR_DEFINITION_NAME, this.testProperties, Collections.singletonList("app.demo.0=foo=bar")); verifyScheduleExistsInScheduler(createScheduleInfo(BASE_SCHEDULE_NAME, CTR_DEFINITION_NAME)); AuditActionType[] createActions = {AuditActionType.CREATE}; diff --git a/whats-new.adoc b/whats-new.adoc index 536a56c1af..312463ad3d 100644 --- a/whats-new.adoc +++ b/whats-new.adoc @@ -43,5 +43,6 @@ VersionInfoProperties versionInfoProperties, SecurityStateBean securityStateBean * * Removed the `DefaultTaskExecutionInfoService` constructor that does not take the `composedTaskRunnerConfigurationProperties` parameter. Use the constructor that offers the `composedTaskRunnerConfigurationProperties` parameter. * AggressiveShutdownStrategy & AggressiveShutdownWithNetworkCleanupStrategy deprecated classes have been removed. Use the KillDownShutdownStrategy class. == Breaking Changes -Announce deprecated changes here +* Deprecated property prefix `scheduler.` has been removed when setting schedule properties for task applications. Use `deployer.` instead. + From f91248e28fbe94dafc9ba7eea46e9c9bcf79feea Mon Sep 17 00:00:00 2001 From: Glenn Renfro Date: Thu, 7 Nov 2024 14:24:50 -0500 Subject: [PATCH 2/2] Remove schedule. prefix from rest client and shell Polish Signed-off-by: Glenn Renfro --- .../server/rest/documentation/BaseDocumentation.java | 2 +- .../rest/documentation/TaskSchedulerDocumentation.java | 2 +- spring-cloud-dataflow-docs/src/main/asciidoc/tasks.adoc | 2 +- .../cloud/dataflow/rest/client/dsl/task/TaskSchedule.java | 2 +- .../dataflow/rest/util/DeploymentPropertiesUtils.java | 8 ++++---- .../rest/util/DeploymentPropertiesUtilsTests.java | 8 ++++---- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/spring-cloud-dataflow-classic-docs/src/test/java/org/springframework/cloud/dataflow/server/rest/documentation/BaseDocumentation.java b/spring-cloud-dataflow-classic-docs/src/test/java/org/springframework/cloud/dataflow/server/rest/documentation/BaseDocumentation.java index 8595da4c78..acaffed639 100644 --- a/spring-cloud-dataflow-classic-docs/src/test/java/org/springframework/cloud/dataflow/server/rest/documentation/BaseDocumentation.java +++ b/spring-cloud-dataflow-classic-docs/src/test/java/org/springframework/cloud/dataflow/server/rest/documentation/BaseDocumentation.java @@ -328,7 +328,7 @@ private List getSampleList() { scheduleInfo.setScheduleName("FOO"); scheduleInfo.setTaskDefinitionName("BAR"); Map props = new HashMap<>(1); - props.put("scheduler.AAA.spring.cloud.scheduler.cron.expression", "00 41 17 ? * *"); + props.put("deployer.AAA.spring.cloud.scheduler.cron.expression", "00 41 17 ? * *"); scheduleInfo.setScheduleProperties(props); result.add(scheduleInfo); return result; diff --git a/spring-cloud-dataflow-classic-docs/src/test/java/org/springframework/cloud/dataflow/server/rest/documentation/TaskSchedulerDocumentation.java b/spring-cloud-dataflow-classic-docs/src/test/java/org/springframework/cloud/dataflow/server/rest/documentation/TaskSchedulerDocumentation.java index ed68795ace..bb8b4fdfae 100644 --- a/spring-cloud-dataflow-classic-docs/src/test/java/org/springframework/cloud/dataflow/server/rest/documentation/TaskSchedulerDocumentation.java +++ b/spring-cloud-dataflow-classic-docs/src/test/java/org/springframework/cloud/dataflow/server/rest/documentation/TaskSchedulerDocumentation.java @@ -64,7 +64,7 @@ void createSchedule() throws Exception { .queryParam("scheduleName", "myschedule") .queryParam("taskDefinitionName", "mytaskname") .queryParam("platform", "default") - .queryParam("properties", "scheduler.cron.expression=00 22 17 ? *") + .queryParam("properties", "deployer.cron.expression=00 22 17 ? *") .queryParam("arguments", "--foo=bar")) .andExpect(status().isCreated()) .andDo(this.documentationHandler.document( diff --git a/spring-cloud-dataflow-docs/src/main/asciidoc/tasks.adoc b/spring-cloud-dataflow-docs/src/main/asciidoc/tasks.adoc index e7970ad798..be7252d1be 100644 --- a/spring-cloud-dataflow-docs/src/main/asciidoc/tasks.adoc +++ b/spring-cloud-dataflow-docs/src/main/asciidoc/tasks.adoc @@ -1346,7 +1346,7 @@ dataflow:>task schedule list ╔══════════════════════════╤════════════════════╤════════════════════════════════════════════════════╗ ║ Schedule Name │Task Definition Name│ Properties ║ ╠══════════════════════════╪════════════════════╪════════════════════════════════════════════════════╣ -║mytaskschedule │mytask │spring.cloud.scheduler.cron.expression = */1 * * * *║ +║mytaskschedule │mytask │spring.cloud.deployer.cron.expression = */1 * * * * ║ ╚══════════════════════════╧════════════════════╧════════════════════════════════════════════════════╝ ---- ==== diff --git a/spring-cloud-dataflow-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/dsl/task/TaskSchedule.java b/spring-cloud-dataflow-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/dsl/task/TaskSchedule.java index 9279f4ce54..ade6996f40 100644 --- a/spring-cloud-dataflow-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/dsl/task/TaskSchedule.java +++ b/spring-cloud-dataflow-rest-client/src/main/java/org/springframework/cloud/dataflow/rest/client/dsl/task/TaskSchedule.java @@ -47,7 +47,7 @@ */ public class TaskSchedule implements AutoCloseable { - public static final String CRON_EXPRESSION_KEY = "scheduler.cron.expression"; + public static final String CRON_EXPRESSION_KEY = "deployer.cron.expression"; private final String scheduleName; diff --git a/spring-cloud-dataflow-rest-resource/src/main/java/org/springframework/cloud/dataflow/rest/util/DeploymentPropertiesUtils.java b/spring-cloud-dataflow-rest-resource/src/main/java/org/springframework/cloud/dataflow/rest/util/DeploymentPropertiesUtils.java index 78af4778bd..6d5f3823f4 100644 --- a/spring-cloud-dataflow-rest-resource/src/main/java/org/springframework/cloud/dataflow/rest/util/DeploymentPropertiesUtils.java +++ b/spring-cloud-dataflow-rest-resource/src/main/java/org/springframework/cloud/dataflow/rest/util/DeploymentPropertiesUtils.java @@ -61,7 +61,7 @@ public final class DeploymentPropertiesUtils { .compile("(\\s(?=" + "([^\\\"']*[\\\"'][^\\\"']*[\\\"'])*[^\\\"']*$))"); - private static final String[] DEPLOYMENT_PROPERTIES_PREFIX ={"deployer", "app", "version", "scheduler", "spring.cloud.dataflow.task"}; + private static final String[] DEPLOYMENT_PROPERTIES_PREFIX ={"deployer", "app", "version", "spring.cloud.dataflow.task"}; private DeploymentPropertiesUtils() { // prevent instantiation @@ -127,7 +127,7 @@ public static List parseParamList(String s, String delimiter) { // we have a key/value pair having '=', or malformed first pair if (!startsWithDeploymentPropertyPrefix(candidate)) { throw new IllegalArgumentException( - "Only deployment property keys starting with 'app.' or 'scheduler' or 'deployer.' or 'version.'" + + "Only deployment property keys starting with 'app.' or 'deployer.' or 'version.'" + " allowed. Not " + candidate); } pairs.add(candidate); @@ -286,9 +286,9 @@ public static void validateDeploymentProperties(Map properties) for (Entry property : properties.entrySet()) { String key = property.getKey(); if (!key.startsWith("app.") && !key.startsWith("deployer.") - && !key.startsWith("scheduler.") && !key.startsWith("version.")) { + && !key.startsWith("version.")) { throw new IllegalArgumentException( - "Only deployment property keys starting with 'app.', 'deployer.' or, 'scheduler.' allowed, got '" + key + "'"); + "Only deployment property keys starting with 'app.' or 'deployer.' allowed, got '" + key + "'"); } } } diff --git a/spring-cloud-dataflow-rest-resource/src/test/java/org/springframework/cloud/dataflow/rest/util/DeploymentPropertiesUtilsTests.java b/spring-cloud-dataflow-rest-resource/src/test/java/org/springframework/cloud/dataflow/rest/util/DeploymentPropertiesUtilsTests.java index a0be9d5437..8edbf5674c 100644 --- a/spring-cloud-dataflow-rest-resource/src/test/java/org/springframework/cloud/dataflow/rest/util/DeploymentPropertiesUtilsTests.java +++ b/spring-cloud-dataflow-rest-resource/src/test/java/org/springframework/cloud/dataflow/rest/util/DeploymentPropertiesUtilsTests.java @@ -52,13 +52,13 @@ private static void assertArrays(String[] left, String[] right) { @Test void deploymentPropertiesParsing() { Map props = DeploymentPropertiesUtils.parse("app.foo.bar=v, app.foo.wizz=v2 , deployer.foo" - + ".pot=fern, app.other.key = value , deployer.other.cow = meww, scheduler.other.key = baz"); + + ".pot=fern, app.other.key = value , deployer.other.cow = meww, deployer.other.key = baz"); assertThat(props.entrySet()).contains(entry("app.foo.bar", "v")); assertThat(props.entrySet()).contains(entry("app.other.key", "value")); assertThat(props.entrySet()).contains(entry("app.foo.wizz", "v2")); assertThat(props.entrySet()).contains(entry("deployer.foo.pot", "fern")); assertThat(props.entrySet()).contains(entry("deployer.other.cow", "meww")); - assertThat(props.entrySet()).contains(entry("scheduler.other.key", "baz")); + assertThat(props.entrySet()).contains(entry("deployer.other.key", "baz")); props = DeploymentPropertiesUtils.parse("app.f=v"); assertThat(props.entrySet()).contains(entry("app.f", "v")); @@ -87,7 +87,7 @@ void deploymentPropertiesParsing() { fail("Illegal Argument Exception expected."); } catch (Exception e) { - assertThat(e.getMessage()).isEqualTo("Only deployment property keys starting with 'app.' or 'scheduler' or 'deployer.' or 'version.' allowed. Not invalidkeyvalue"); + assertThat(e.getMessage()).isEqualTo("Only deployment property keys starting with 'app.' or 'deployer.' or 'version.' allowed. Not invalidkeyvalue"); } props = DeploymentPropertiesUtils.parse("deployer.foo=bar,invalidkeyvalue2"); @@ -125,7 +125,7 @@ void deploymentPropertiesParsing2() { fail("Illegal Argument Exception expected."); } catch (Exception e) { - assertThat(e.getMessage()).isEqualTo("Only deployment property keys starting with 'app.' or 'scheduler' or 'deployer.' or 'version.' allowed. Not a=b"); + assertThat(e.getMessage()).isEqualTo("Only deployment property keys starting with 'app.' or 'deployer.' or 'version.' allowed. Not a=b"); } props = DeploymentPropertiesUtils.parseArgumentList("a=b c=d", " ");