diff --git a/digdag-core/src/main/java/io/digdag/core/database/DatabaseProjectStoreManager.java b/digdag-core/src/main/java/io/digdag/core/database/DatabaseProjectStoreManager.java index 5e69763892..99c372d6b4 100644 --- a/digdag-core/src/main/java/io/digdag/core/database/DatabaseProjectStoreManager.java +++ b/digdag-core/src/main/java/io/digdag/core/database/DatabaseProjectStoreManager.java @@ -114,6 +114,15 @@ public StoredRevision getRevisionOfWorkflowDefinition(long wfId) "revision of workflow definition id=%s", wfId); } + private static String makeLastIdCond(Optional lastId, boolean ascending) + { + String signIneq = ascending ? "\\>" : "\\<"; + Long lastIdValue = lastId.or( () -> ascending ? 0L : Long.MAX_VALUE); + return String.format("%s %d", signIneq, lastIdValue); + } + + + private class DatabaseProjectStore implements ProjectStore { @@ -294,15 +303,21 @@ public StoredWorkflowDefinitionWithProject getLatestWorkflowDefinitionByName(int public List getLatestActiveWorkflowDefinitions( int pageSize, Optional lastId, + boolean ascending, Optional namePattern, + boolean searchProjectName, AccessController.ListFilter acFilter) throws ResourceNotFoundException { + String projectNamePattern = searchProjectName ? generatePartialMatchPattern(namePattern) : ""; + String ascDesc = ascending ? "asc" : "desc"; return autoCommit((handle, dao) -> dao.getLatestActiveWorkflowDefinitions( siteId, pageSize, - lastId.or(0L), + makeLastIdCond(lastId, ascending), generatePartialMatchPattern(namePattern), + projectNamePattern, + ascDesc, acFilter.getSql()) ); } @@ -617,21 +632,23 @@ List getProjectsWithLatestRevision( " join revisions rev on a.revision_id = rev.id" + " join projects proj on a.project_id = proj.id" + " join workflow_configs wc on wc.id = wd.config_id" + - " where wd.id \\> :lastId" + + " where wd.id " + // `workflow_definitions` table has a composite index // for `revision_id` and `name` (`workflow_definitions_on_revision_id_and_name`). // And the index is used for filter by `revision_id` and `name`. // Since this query always limits the records by `revision_id` (the latest revision's one), // partial matching of `name` (e.g. '%test%') can be accepted. - " and wd.name like :namePattern" + + " and ( wd.name like :namePattern or proj.name like :projectNamePattern )" + " and " + - " order by wd.id" + + " order by wd.id " + " limit :limit") List getLatestActiveWorkflowDefinitions( @Bind("siteId") int siteId, @Bind("limit") int limit, - @Bind("lastId") long lastId, + @Define("lastIdCond") String lastIdCond, @Bind("namePattern") String namePattern, + @Bind("projectNamePattern") String projectNamePattern, + @Define("orderDirection") String orderDirection, @Define("acFilter") String acFilter); } @@ -688,26 +705,28 @@ List getProjectsWithLatestRevision( " and p.deleted_at is null" + " group by r.project_id" + " )) " + - " and wf.id \\> :lastId" + + " and wf.id " + // `workflow_definitions` table has a composite index // for `revision_id` and `name` (`workflow_definitions_on_revision_id_and_name`). // And the index is used for filter by `revision_id` and `name`. // Since this query always limits the records by `revision_id` (the latest revision's one), // partial matching of `name` (e.g. '%test%') can be accepted. - " and wf.name like :namePattern" + + " and ( wf.name like :namePattern or proj.name like :projectNamePattern )" + " and " + - " order by wf.id" + + " order by wf.id " + " limit :limit" + ") wd" + " join revisions r on r.id = wd.revision_id" + " join projects p on p.id = r.project_id" + " join workflow_configs wc on wc.id = wd.config_id" + - " order by wd.id") + " order by wd.id ") List getLatestActiveWorkflowDefinitions( @Bind("siteId") int siteId, @Bind("limit") int limit, - @Bind("lastId") long lastId, + @Define("lastIdCond") String lastIdCond, @Bind("namePattern") String namePattern, + @Bind("projectNamePattern") String projectNamePattern, + @Define("orderDirection") String orderDirection, @Define("acFilter") String acFilter); } @@ -820,11 +839,27 @@ List getProjectsWithLatestRevision( " limit 1") StoredWorkflowDefinitionWithProject getLatestWorkflowDefinitionByName(@Bind("siteId") int siteId, @Bind("projId") int projId, @Bind("name") String name); - List getLatestActiveWorkflowDefinitions(int siteId, int limit, long lastId, String namePattern, String acFilter); + default List getLatestActiveWorkflowDefinitions(int siteId, int limit, long lastId, String namePattern, String acFilter) + { + // projects.name must be non-empty or null(for deleted projects). So empty string "" will never match. + return getLatestActiveWorkflowDefinitions(siteId, limit, makeLastIdCond(Optional.of(lastId), true), namePattern, "", "asc", acFilter); + } + + /** + * + * @param siteId Target site_id + * @param limit Number of workflows to be returned + * @param lastIdCond Pagination based on workflow id. Must {@literal "> n" or "< n}" + * @param namePattern Search by workflow name with partial match. namePattern and projectNamePattern is "OR" search. + * @param projectNamePattern Search by project name with partial match. namePattern and projectNamePattern is "OR" search. + * @param orderDirection Order based on workflow id. "asc" or "desc". The parameter must be validated before calling to avoid SQL injection. + * @param acFilter AccessControl filter clause. The parameter must be validated before calling to avoid SQL injection. + * @return + */ + List getLatestActiveWorkflowDefinitions(int siteId, int limit, String lastIdCond, String namePattern, String projectNamePattern, String orderDirection, String acFilter); // getWorkflowDetailsById is same with getWorkflowDetailsByIdInternal // excepting site_id check - @SqlQuery("select wd.*, wc.config, wc.timezone," + " proj.id as proj_id, proj.name as proj_name, proj.deleted_name as proj_deleted_name, proj.deleted_at as proj_deleted_at, proj.site_id, proj.created_at as proj_created_at," + " rev.name as rev_name, rev.default_params as rev_default_params" + diff --git a/digdag-core/src/main/java/io/digdag/core/repository/ProjectStore.java b/digdag-core/src/main/java/io/digdag/core/repository/ProjectStore.java index ff32345c8c..6d2ffd04b8 100644 --- a/digdag-core/src/main/java/io/digdag/core/repository/ProjectStore.java +++ b/digdag-core/src/main/java/io/digdag/core/repository/ProjectStore.java @@ -64,7 +64,18 @@ StoredWorkflowDefinitionWithProject getWorkflowDefinitionById(long wfId) StoredWorkflowDefinitionWithProject getLatestWorkflowDefinitionByName(int projId, String name) throws ResourceNotFoundException; - List getLatestActiveWorkflowDefinitions(int pageSize, Optional lastId, Optional namePattern, AccessController.ListFilter acFilter) + // For backward compatibility + default List getLatestActiveWorkflowDefinitions( + int pageSize, Optional lastId, Optional namePattern, AccessController.ListFilter acFilter) + throws ResourceNotFoundException + { + // ascending is true and searchProjectName is disabled. + return getLatestActiveWorkflowDefinitions(pageSize, lastId, true, namePattern, false, acFilter); + } + + List getLatestActiveWorkflowDefinitions( + int pageSize, Optional lastId, boolean ascending, Optional namePattern, + boolean searchProjectName, AccessController.ListFilter acFilter) throws ResourceNotFoundException; TimeZoneMap getWorkflowTimeZonesByIdList(List defIdList); diff --git a/digdag-core/src/test/java/io/digdag/core/database/DatabaseProjectStoreManagerTest.java b/digdag-core/src/test/java/io/digdag/core/database/DatabaseProjectStoreManagerTest.java index e6c7e712c3..8fbc3b26a4 100644 --- a/digdag-core/src/test/java/io/digdag/core/database/DatabaseProjectStoreManagerTest.java +++ b/digdag-core/src/test/java/io/digdag/core/database/DatabaseProjectStoreManagerTest.java @@ -375,6 +375,12 @@ public void testGetActiveWorkflows() store.getLatestActiveWorkflowDefinitions(100, Optional.absent(), Optional.fromNullable("%_"), () -> "true")); assertEquals(ImmutableList.of(), store.getLatestActiveWorkflowDefinitions(100, Optional.absent(), Optional.fromNullable("*"), () -> "true")); + // Check ascending parameter. + assertEquals(ImmutableList.of(wfDetails6, wfDetails5, wfDetails4, wfDetails3, wfDetails2, wfDetails1), + store.getLatestActiveWorkflowDefinitions(100, Optional.absent(), false, Optional.absent(), false, () -> "true")); + // Check searchProjectName parameter. + assertEquals(ImmutableList.of(wfDetails6, wfDetails5, wfDetails4, wfDetails3, wfDetails2, wfDetails1), + store.getLatestActiveWorkflowDefinitions(100, Optional.absent(), false, Optional.fromNullable("proj1"), true, () -> "true")); }); } diff --git a/digdag-server/src/main/java/io/digdag/server/rs/WorkflowResource.java b/digdag-server/src/main/java/io/digdag/server/rs/WorkflowResource.java index ffe504c6fd..7c06458990 100644 --- a/digdag-server/src/main/java/io/digdag/server/rs/WorkflowResource.java +++ b/digdag-server/src/main/java/io/digdag/server/rs/WorkflowResource.java @@ -7,6 +7,7 @@ import java.time.ZonedDateTime; import java.time.temporal.ChronoUnit; +import javax.ws.rs.DefaultValue; import javax.ws.rs.Produces; import javax.ws.rs.Path; import javax.ws.rs.PathParam; @@ -110,23 +111,30 @@ public RestWorkflowDefinition getWorkflowDefinition( @Path("/api/workflows") @ApiOperation("List workflows") public RestWorkflowDefinitionCollection getWorkflowDefinitions( - @ApiParam(value="list workflows whose id is grater than this id for pagination", required=false) + @ApiParam(value="pagination. return workflows which id are greater than last_id with order 'asc', which id are less than the last_id with order 'desc'", required=false) @QueryParam("last_id") Long lastId, @ApiParam(value="number of workflows to return", required=false) @QueryParam("count") Integer count, + @ApiParam(value="Sort order. 'asc' or 'desc'", defaultValue = "asc", required=false) + @DefaultValue("asc") @QueryParam("order") String orderDirection, @ApiParam(value="name pattern to be partially matched", required=false) - @QueryParam("name_pattern") String namePattern + @QueryParam("name_pattern") String namePattern, + @ApiParam(value="name pattern to be partially matched", required=false) + @DefaultValue("false") @QueryParam("search_project_name") Boolean searchProjectName ) throws ResourceNotFoundException, AccessControlException { final SiteTarget siteTarget = SiteTarget.of(getSiteId()); ac.checkListWorkflowsOfSite(siteTarget, getAuthenticatedUser()); // AccessControl - return tm.begin(() -> { List defs = rm.getProjectStore(getSiteId()) - .getLatestActiveWorkflowDefinitions(Optional.fromNullable(count).or(100), Optional.fromNullable(lastId), // check NotFound first + .getLatestActiveWorkflowDefinitions( + Optional.fromNullable(count).or(100), + Optional.fromNullable(lastId), // check NotFound first + orderAscending(orderDirection), Optional.fromNullable(namePattern), + searchProjectName, ac.getListWorkflowsFilterOfSite( SiteTarget.of(getSiteId()), getAuthenticatedUser())); @@ -135,6 +143,19 @@ public RestWorkflowDefinitionCollection getWorkflowDefinitions( }, ResourceNotFoundException.class, AccessControlException.class); } + private boolean orderAscending(String orderDirection) + { + if (orderDirection == null || orderDirection.equals("asc")) { + return true; + } + else if (orderDirection.equals("desc")) { + return false; + } + else { + throw new IllegalArgumentException("parameter 'order' must be either 'asc' or 'desc'"); + } + } + @DigdagTimed(category = "api", value = "getWorkflowDefinitionById") @GET @Path("/api/workflows/{id}") diff --git a/digdag-tests/src/test/java/acceptance/ApiWorkflowsIT.java b/digdag-tests/src/test/java/acceptance/ApiWorkflowsIT.java new file mode 100644 index 0000000000..e22ef3acef --- /dev/null +++ b/digdag-tests/src/test/java/acceptance/ApiWorkflowsIT.java @@ -0,0 +1,143 @@ +package acceptance; + +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.InjectableValues; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.guava.GuavaModule; +import io.digdag.client.api.JacksonTimeModule; +import io.digdag.client.api.RestWorkflowDefinitionCollection; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import okhttp3.Response; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import utils.CommandStatus; +import utils.TemporaryDigdagServer; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.stream.Collectors; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static utils.TestUtils.copyResource; +import static utils.TestUtils.main; + +public class ApiWorkflowsIT +{ + @Rule + public TemporaryFolder folder = new TemporaryFolder(); + + @Rule + public TemporaryDigdagServer server = TemporaryDigdagServer.of(); + + private Path config; + private Path projectDir; + private ObjectMapper objectMapper; + + @Before + public void setUp() + throws Exception + { + objectMapper = new ObjectMapper() + .registerModule(new GuavaModule()) + .registerModule(new JacksonTimeModule()) + .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + // To deserialize Config class, need ObjectMapper. + objectMapper.setInjectableValues(new InjectableValues.Std().addValue(ObjectMapper.class, objectMapper)); + + projectDir = folder.getRoot().toPath().resolve("foobar"); + Files.createDirectory(projectDir); + config = folder.newFile().toPath(); + + copyResource("acceptance/basic.dig", projectDir.resolve("wf1abc.dig")); + copyResource("acceptance/basic.dig", projectDir.resolve("wf2def.dig")); + + // Push first project. (named: prj1foo) + { + CommandStatus pushStatus = main( + "push", + "--project", projectDir.toString(), + "prj1foo", + "-c", config.toString(), + "-e", server.endpoint()); + assertThat(pushStatus.errUtf8(), pushStatus.code(), is(0)); + } + + // Push second project. (named: prj2bar) + { + CommandStatus pushStatus = main( + "push", + "--project", projectDir.toString(), + "prj2bar", + "-c", config.toString(), + "-e", server.endpoint()); + assertThat(pushStatus.errUtf8(), pushStatus.code(), is(0)); + } + } + + @Test + public void showWorkflows() + throws Exception + { + { // No params + RestWorkflowDefinitionCollection restWorkflows = callGetWorkflows(""); + assertThat("Num of workflows", restWorkflows.getWorkflows().size(), is(4)); + String ids = restWorkflows.getWorkflows().stream().map((n) -> { + return n.getId().get(); + }).collect(Collectors.joining(",")); + assertThat("List of workflow ids", ids, is("1,2,3,4")); + } + { // order=desc count=2 + RestWorkflowDefinitionCollection restWorkflows = callGetWorkflows("?order=desc&count=2"); + assertThat("Num of workflows", restWorkflows.getWorkflows().size(), is(2)); + String ids = restWorkflows.getWorkflows().stream().map((n) -> { + return n.getId().get(); + }).collect(Collectors.joining(",")); + assertThat("List of workflow ids", ids, is("4,3")); + } + { // order=desc last_id=3 + RestWorkflowDefinitionCollection restWorkflows = callGetWorkflows("?order=desc&last_id=3"); + assertThat("Num of workflows", restWorkflows.getWorkflows().size(), is(2)); + String ids = restWorkflows.getWorkflows().stream().map((n) -> { + return n.getId().get(); + }).collect(Collectors.joining(",")); + assertThat("List of workflow ids", ids, is("2,1")); + } + { // order=desc count=2 name_pattern=f1a + RestWorkflowDefinitionCollection restWorkflows = callGetWorkflows("?order=desc&count=2&name_pattern=f1a"); + assertThat("Num of workflows", restWorkflows.getWorkflows().size(), is(2)); + String ids = restWorkflows.getWorkflows().stream().map((n) -> { + return n.getId().toString() + "-" + n.getName(); + }).collect(Collectors.joining(",")); + assertThat("List of workflows", ids, is("3-wf1abc,1-wf1abc")); + } + + { // order=desc count=2 name_pattern=f1a search_project_name=true + RestWorkflowDefinitionCollection restWorkflows = callGetWorkflows("?order=desc&count=2&name_pattern=prj2&search_project_name=true"); + assertThat("Num of workflows", restWorkflows.getWorkflows().size(), is(2)); + String ids = restWorkflows.getWorkflows().stream().map((n) -> { + return n.getId().toString() + "-" + n.getProject().getName() + "-" + n.getName(); + }).collect(Collectors.joining(",")); + assertThat("List of workflows", ids, is("4-prj2bar-wf2def,3-prj2bar-wf1abc")); + } + } + + private RestWorkflowDefinitionCollection callGetWorkflows(String queryParams) + throws IOException + { + OkHttpClient client = new OkHttpClient(); + + Response response = client.newCall(new Request.Builder() + .url(server.endpoint() + "/api/workflows" + queryParams) + .build()).execute(); + assertThat("Response failed.", response.isSuccessful(), is(true)); + JsonNode json = objectMapper.readTree(response.body().string()); + RestWorkflowDefinitionCollection restWorkflows = objectMapper.treeToValue(json, RestWorkflowDefinitionCollection.class); + return restWorkflows; + } +}