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

Update Task Thin Executions list function for parity with 2.11.x #6063

Closed
wants to merge 6 commits into from
Closed

Update Task Thin Executions list function for parity with 2.11.x #6063

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
364414b
Update Task Thin Executions list function for parity with 2.11.x
corneil Nov 12, 2024
3f80045
Update Task Thin Executions list function for parity with 2.11.x
corneil Nov 12, 2024
2e8f708
Update Task Thin Executions list function for parity with 2.11.x
corneil Nov 12, 2024
d0a457e
Update Task Thin Executions list function for parity with 2.11.x
corneil Nov 12, 2024
37529f6
Revised TaskTemplate to retain order of link init in the constructor.
corneil Nov 13, 2024
2c21343
Update Task Thin Executions list function for parity with 2.11.x
corneil Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...t/java/org/springframework/cloud/dataflow/server/rest/documentation/ApiDocumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ void index() throws Exception {
fieldWithPath("_links.tasks/thinexecutions.href").description("Link to the tasks/thinexecutions"),

fieldWithPath("_links.tasks/thinexecutions/name.href").description("Link to the tasks/thinexecutions/name"),
fieldWithPath("_links.tasks/thinexecutions/name.templated").description("Link to the tasks/thinexecutions/name is templated"),
fieldWithPath("_links.tasks/thinexecutions/name.templated").type(JsonFieldType.BOOLEAN).optional().description("Link to the tasks/thinexecutions/name is templated"),

fieldWithPath("_links.tasks/info/executions.href").description("Link to the tasks/info/executions"),
fieldWithPath("_links.tasks/info/executions.templated").type(JsonFieldType.BOOLEAN).optional().description("Link tasks/info is templated"),
Expand Down
76 changes: 19 additions & 57 deletions ...est-client/src/main/java/org/springframework/cloud/dataflow/rest/client/TaskTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,8 @@ public class TaskTemplate implements TaskOperations {

private static final String DEFINITION_RELATION = "tasks/definitions/definition";

private static final String EXECUTIONS_CURRENT_RELATION_VERSION = "1.7.0";
private static final String VALIDATION_MIN_VERSION = "3.0.0-SNAPSHOT";

private static final String VALIDATION_RELATION_VERSION = "1.7.0";
private static final String VALIDATION_THIN_TASK_VERSION = "2.11.3";
private static final String EXECUTIONS_RELATION = "tasks/executions";

private static final String THIN_EXECUTIONS_RELATION = "tasks/thinexecutions";
Expand Down Expand Up @@ -135,50 +133,37 @@ public class TaskTemplate implements TaskOperations {
EXECUTIONS_INFO_RELATION,
PLATFORM_LIST_RELATION,
RETRIEVE_LOG,
VALIDATION_REL
VALIDATION_REL,
EXECUTIONS_CURRENT_RELATION,
THIN_EXECUTIONS_RELATION,
THIN_EXECUTIONS_BY_NAME_RELATION,
EXECUTION_LAUNCH_RELATION
).forEach(relation -> {
Assert.isTrue(resources.getLink(relation).isPresent(), () -> relation + " relation is required");
});
this.dataFlowServerVersion = dataFlowServerVersion;
this.restTemplate = restTemplate;

String version = VersionUtils.getThreePartVersion(dataFlowServerVersion);
if (VersionUtils.isDataFlowServerVersionGreaterThanOrEqualToRequiredVersion(version, VALIDATION_RELATION_VERSION)) {
Assert.notNull(resources.getLink(VALIDATION_REL), ()-> VALIDATION_REL + " relation is required");
if (StringUtils.hasText(version)) {
Assert.isTrue(
VersionUtils.isDataFlowServerVersionGreaterThanOrEqualToRequiredVersion(version,
VALIDATION_MIN_VERSION),
() -> "Minimum Data Flow version required is " + VALIDATION_MIN_VERSION + " but got " + version);
}
if (VersionUtils.isDataFlowServerVersionGreaterThanOrEqualToRequiredVersion(version, EXECUTIONS_CURRENT_RELATION_VERSION)) {
Assert.isTrue(resources.getLink(EXECUTIONS_CURRENT_RELATION).isPresent(), ()-> EXECUTIONS_CURRENT_RELATION + " relation is required");
this.executionsCurrentLink = resources.getLink(EXECUTIONS_CURRENT_RELATION).get();
} else {
this.executionsCurrentLink = null;
}

this.executionsCurrentLink = resources.getLink(EXECUTIONS_CURRENT_RELATION).get();
this.aboutLink = resources.getLink("about").get();

this.definitionsLink = resources.getLink(DEFINITIONS_RELATION).get();
this.definitionLink = resources.getLink(DEFINITION_RELATION).get();
this.executionsLink = resources.getLink(EXECUTIONS_RELATION).get();
this.executionLink = resources.getLink(EXECUTION_RELATION).get();
if(resources.getLink(THIN_EXECUTIONS_RELATION).isPresent()) {
this.thinExecutionsLink = resources.getLink(THIN_EXECUTIONS_RELATION).get();
} else {
this.thinExecutionsLink = null;
}
if(resources.getLink(THIN_EXECUTIONS_BY_NAME_RELATION).isPresent()) {
this.thinExecutionsByNameLink = resources.getLink(THIN_EXECUTIONS_BY_NAME_RELATION).get();
} else {
this.thinExecutionsByNameLink = null;
}

if(resources.getLink(EXECUTION_LAUNCH_RELATION).isPresent()) {
this.executionLaunchLink = resources.getLink(EXECUTION_LAUNCH_RELATION).get();
} else {
this.executionLaunchLink = null;
}
this.thinExecutionsLink = resources.getLink(THIN_EXECUTIONS_RELATION).get();
this.thinExecutionsByNameLink = resources.getLink(THIN_EXECUTIONS_BY_NAME_RELATION).get();
this.executionLaunchLink = resources.getLink(EXECUTION_LAUNCH_RELATION).get();
this.executionByNameLink = resources.getLink(EXECUTION_RELATION_BY_NAME).get();
if (resources.getLink(EXECUTIONS_INFO_RELATION).isPresent()) {
this.executionsInfoLink = resources.getLink(EXECUTIONS_INFO_RELATION).get();
}
this.executionsInfoLink = resources.getLink(EXECUTIONS_INFO_RELATION).get();
this.validationLink = resources.getLink(VALIDATION_REL).get();
this.platformListLink = resources.getLink(PLATFORM_LIST_RELATION).get();
this.retrieveLogLink = resources.getLink(RETRIEVE_LOG).get();
Expand Down Expand Up @@ -207,39 +192,16 @@ public TaskDefinitionResource create(String name, String definition, String desc
return restTemplate.postForObject(definitionsLink.expand().getHref(), values,
TaskDefinitionResource.class);
}
private boolean isNewServer() {
if(this.actualDataFlowServerCoreVersion == null) {
AboutResource aboutResource = restTemplate.getForObject(aboutLink.expand().getHref(), AboutResource.class);
Assert.notNull(aboutResource, "Expected about");
this.actualDataFlowServerCoreVersion = aboutResource.getVersionInfo().getCore().getVersion();
}
String v2_11_0 = VersionUtils.getThreePartVersion("2.11.0-SNAPSHOT");
String serverVersion = VersionUtils.getThreePartVersion(this.actualDataFlowServerCoreVersion);
return VersionUtils.isDataFlowServerVersionGreaterThanOrEqualToRequiredVersion(serverVersion, v2_11_0);
}

@Override
public LaunchResponseResource launch(String name, Map<String, String> properties, List<String> arguments) {
MultiValueMap<String, Object> values = new LinkedMultiValueMap<>();
String formattedProperties = DeploymentPropertiesUtils.format(properties);
String commandLineArguments = StringUtils.collectionToDelimitedString(arguments, " ");
values.add("properties", formattedProperties);
values.add("arguments", commandLineArguments);
if(isNewServer()) {
Assert.notNull(executionLaunchLink, "This version of SCDF doesn't support tasks/executions/launch");
values.add("name", name);
String url = executionLaunchLink.expand(name).getHref();
values.remove("name");
return restTemplate.postForObject(url, values, LaunchResponseResource.class);
} else {
Long id = restTemplate.postForObject(executionByNameLink.expand(name).getHref(), values, Long.class, name);
if(id != null) {
LaunchResponseResource response = new LaunchResponseResource();
response.setExecutionId(id);
return response;
} else {
throw new RuntimeException("Expected id");
}
}
String url = executionLaunchLink.expand(name).getHref();
return restTemplate.postForObject(url, values, LaunchResponseResource.class);
}

@Override
Expand Down
20 changes: 6 additions & 14 deletions ...lient/src/test/java/org/springframework/cloud/dataflow/rest/client/TaskTemplateTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.springframework.web.client.RestTemplate;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.Mockito.mock;

/**
Expand All @@ -46,22 +47,19 @@ class TaskTemplateTests {
void setup() {
restTemplate = mock(RestTemplate.class);
}

@Test
void oldDataFlow() {
validateExecutionLinkNotPresent("1.6.0");
void invalidVersion() {
assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> validateExecutionLinkPresent("2.11.5"));
}

@Test
void minDataFlow() {
validateExecutionLinkPresent("1.7.0");
validateExecutionLinkPresent("3.0.0");
cppwfs marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
void futureDataFlow() {
validateExecutionLinkPresent("1.8.0");
validateExecutionLinkPresent("1.9.0");
validateExecutionLinkPresent("2.0.0");
validateExecutionLinkPresent("3.0.0");
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want to have negative test as well. i.e. test with an old version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment we don't have old versions we support with links missing

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking of something like:

	@Test
	void invalidDataFlow() {
		assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> validateExecutionLinkPresent("2.11.5"));
	}
	```

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

}


Expand All @@ -71,12 +69,6 @@ private void validateExecutionLinkPresent(String dataFlowVersion) {
assertThat(testResource.isLinkRequested(CURRENT_TASK_EXECUTION_LINK)).isTrue();
}

private void validateExecutionLinkNotPresent(String version) {
TestResource testResource = new TestResource();
new TaskTemplate(this.restTemplate, testResource, version);
assertThat(testResource.isLinkRequested(CURRENT_TASK_EXECUTION_LINK)).isFalse();
}

public static class TestResource extends RepresentationModel<TestResource> {

private final Map<String, Long> linksRequested = new HashMap<>();
Expand Down
6 changes: 6 additions & 0 deletions ...va/org/springframework/cloud/dataflow/server/controller/TaskExecutionControllerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,12 @@ void getAllThinExecutions() throws Exception {
}
@Test
void getThinExecutionsByName() throws Exception {
mockMvc.perform(get("/tasks/thinexecutions").queryParam("name", TASK_NAME_ORIG).accept(MediaType.APPLICATION_JSON))
.andExpect(status().isOk())
.andExpect(jsonPath("$._embedded.taskExecutionThinResourceList[*].executionId", containsInAnyOrder(2, 1)))
.andExpect(jsonPath("$._embedded.taskExecutionThinResourceList[*].parentExecutionId", containsInAnyOrder(null, 1)))
.andExpect(jsonPath("$._embedded.taskExecutionThinResourceList[*].taskExecutionStatus", containsInAnyOrder("RUNNING", "RUNNING")))
.andExpect(jsonPath("$._embedded.taskExecutionThinResourceList", hasSize(2)));
mockMvc.perform(get("/tasks/thinexecutions").queryParam("name", "nope").accept(MediaType.APPLICATION_JSON))
.andExpect(jsonPath("$.page.totalElements", is(0)));
mockMvc.perform(get("/tasks/thinexecutions").queryParam("name", "none").accept(MediaType.APPLICATION_JSON))
Expand Down
Loading