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

Deprecate search field for taskName search in TaskDefinitionController #6009

Closed
wants to merge 1 commit into from
Closed

Deprecate search field for taskName search in TaskDefinitionController #6009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions ...pringframework/cloud/dataflow/server/rest/documentation/TaskDefinitionsDocumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,15 @@ void listAllTaskDefinitions() throws Exception {
.queryParam("page", "0")
.queryParam("size", "10")
.queryParam("sort", "taskName,ASC")
.queryParam("search", "")
.queryParam("taskName", "")
.queryParam("manifest", "true")
)
.andExpect(status().isOk())
.andDo(this.documentationHandler.document(
queryParameters(
parameterWithName("page").description("The zero-based page number (optional)"),
parameterWithName("size").description("The requested page size (optional)"),
parameterWithName("search").description("The search string performed on the name (optional)"),
parameterWithName("taskName").description("Search for a specific taskName (optional)"),
parameterWithName("sort").description("The sort on the list (optional)"),
parameterWithName("manifest").description("The flag to include the task manifest into the latest task execution (optional)")
),
Expand Down
8 changes: 4 additions & 4 deletions spring-cloud-dataflow-docs/src/main/asciidoc/api-guide.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1526,7 +1526,7 @@ The task definition endpoint lets you get all task definitions.
The following topics provide more details:

* <<api-guide-resources-stream-task-definitions-list-request-structure>>
* <<api-guide-resources-stream-task-definitions-list-request-parameters>>
* <<api-guide-resources-stream-task-definitions-list-query-parameters>>
* <<api-guide-resources-stream-task-definitions-list-example-request>>
* <<api-guide-resources-stream-task-definitions-list-response-structure>>

Expand All @@ -1539,10 +1539,10 @@ include::{snippets}/task-definitions-documentation/list-all-task-definitions/htt



[[api-guide-resources-stream-task-definitions-list-request-parameters]]
===== Request Parameters
[[api-guide-resources-stream-task-definitions-list-query-parameters]]
===== Query Parameters

include::{snippets}/task-definitions-documentation/list-all-task-definitions/request-parameters.adoc[]
include::{snippets}/task-definitions-documentation/list-all-task-definitions/query-parameters.adoc[]



Expand Down
8 changes: 2 additions & 6 deletions ...n/java/org/springframework/cloud/dataflow/server/controller/TaskDefinitionController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ public void destroyAll() {
* Return a page-able list of {@link TaskDefinitionResource} defined tasks.
*
* @param pageable page-able collection of {@code TaskDefinitionResource}
* @param search optional findByTaskNameContains parameter (Deprecated: please use taskName instead)
* @param taskName optional findByTaskNameContains parameter
* @param dslText optional findByDslText parameter
* @param description optional findByDescription parameter
Expand All @@ -172,7 +171,6 @@ public void destroyAll() {
@ResponseStatus(HttpStatus.OK)
public PagedModel<? extends TaskDefinitionResource> list(
Pageable pageable,
@RequestParam(required = false) @Deprecated String search,
@RequestParam(required = false) String taskName,
@RequestParam(required = false) String description,
@RequestParam(required = false) boolean manifest,
Expand All @@ -181,14 +179,12 @@ public PagedModel<? extends TaskDefinitionResource> list(
) {
final Page<TaskDefinition> taskDefinitions;

if (Stream.of(search, taskName, description, dslText).filter(Objects::nonNull).count() > 1L) {
throw new TaskQueryParamException(new String[]{"taskName (or search)", "description", "dslText"});
if (Stream.of(taskName, description, dslText).filter(Objects::nonNull).count() > 1L) {
throw new TaskQueryParamException(new String[]{"taskName", "description", "dslText"});
}

if (taskName != null) {
taskDefinitions = repository.findByTaskNameContains(taskName, pageable);
} else if (search != null) {
taskDefinitions = repository.findByTaskNameContains(search, pageable);
} else if (description != null) {
taskDefinitions = repository.findByDescriptionContains(description, pageable);
} else if (dslText != null) {
Expand Down
14 changes: 7 additions & 7 deletions ...c/test/java/org/springframework/cloud/dataflow/server/controller/TaskControllerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,28 +334,28 @@ void saveCompositeTaskWithParameters() throws Exception {
assertThat(myTask2.getName()).isEqualTo("myTask-t2");
}

@ParameterizedTest
@ValueSource(strings = {"search", "taskName"})
void findTaskNameContainsSubstring(String taskNameRequestParamName) throws Exception {
@Test
void findTaskNameContainsSubstring() throws Exception {
final String TASK_NAME_REQUEST_PARAMETER = "taskName";
Copy link
Contributor

Choose a reason for hiding this comment

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

[TinyNit] Please remove the final keyword as we only use them where it adds value (like immutable fields etc..)

repository.save(new TaskDefinition("foo", "task"));
repository.save(new TaskDefinition("foz", "task"));
repository.save(new TaskDefinition("ooz", "task"));

mockMvc.perform(get("/tasks/definitions").param(taskNameRequestParamName, "f")
mockMvc.perform(get("/tasks/definitions").param(TASK_NAME_REQUEST_PARAMETER, "f")
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isOk())
.andExpect(jsonPath("$._embedded.taskDefinitionResourceList.*", hasSize(2)))

.andExpect(jsonPath("$._embedded.taskDefinitionResourceList[0].name", is("foo")))
.andExpect(jsonPath("$._embedded.taskDefinitionResourceList[1].name", is("foz")));

mockMvc.perform(get("/tasks/definitions").param(taskNameRequestParamName, "oz")
mockMvc.perform(get("/tasks/definitions").param(TASK_NAME_REQUEST_PARAMETER, "oz")
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isOk())
.andExpect(jsonPath("$._embedded.taskDefinitionResourceList.*", hasSize(2)))

.andExpect(jsonPath("$._embedded.taskDefinitionResourceList[0].name", is("foz")))
.andExpect(jsonPath("$._embedded.taskDefinitionResourceList[1].name", is("ooz")));

mockMvc.perform(get("/tasks/definitions").param(taskNameRequestParamName, "o")
mockMvc.perform(get("/tasks/definitions").param(TASK_NAME_REQUEST_PARAMETER, "o")
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isOk())
.andExpect(jsonPath("$._embedded.taskDefinitionResourceList.*", hasSize(3)))

Expand Down Expand Up @@ -411,7 +411,7 @@ void findDslTextContainsSubstring() throws Exception {

@Test
void findByDslTextAndNameBadRequest() throws Exception {
mockMvc.perform(get("/tasks/definitions").param("dslText", "fo").param("search", "f")
mockMvc.perform(get("/tasks/definitions").param("dslText", "fo").param("taskName", "f")
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case we reference the param by string name rather than constant. I would be in favor of moving the TASK_NAME_REQUEST_PARAMETER to a static final constant or just using "taskName" everywhere.

.accept(MediaType.APPLICATION_JSON)).andExpect(status().isBadRequest());
}

Expand Down
Loading