Skip to content

Commit

Permalink
Deprecate search field for taskName search in TaskDefinitionController
Browse files Browse the repository at this point in the history
resolves #5993

* Remove search as a parameter from the javadoc
* Fix docs such that queryParameter descriptions appear in reference docs.
* Rename Request Parameters header  to Query Parameters

Signed-off-by: Glenn Renfro <grenfro@vmware.com>

Applied code review changes

Signed-off-by: Glenn Renfro <grenfro@vmware.com>
  • Loading branch information
cppwfs committed Oct 24, 2024
1 parent 78085ce commit a30ee55
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 16 deletions.
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
Original file line number Diff line number Diff line change
Expand Up @@ -1542,7 +1542,7 @@ include::{snippets}/task-definitions-documentation/list-all-task-definitions/htt
[[api-guide-resources-stream-task-definitions-list-request-parameters]]
===== Request 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
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
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@
@AutoConfigureTestDatabase(replace = Replace.ANY)
class TaskControllerTests {

private final static String TASK_NAME_REQUEST_PARAMETER = "taskName";

@Autowired
TaskDefinitionAssemblerProvider taskDefinitionAssemblerProvider;

Expand Down Expand Up @@ -334,28 +336,27 @@ 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 {
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 +412,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(TASK_NAME_REQUEST_PARAMETER, "f")
.accept(MediaType.APPLICATION_JSON)).andExpect(status().isBadRequest());
}

Expand Down

0 comments on commit a30ee55

Please sign in to comment.