From 52e5fe617a7f2515e5c054432653075c7518371d Mon Sep 17 00:00:00 2001 From: "Heiko W. Rupp" Date: Tue, 9 Jun 2020 18:14:00 +0200 Subject: [PATCH 1/5] POL-293 Add paging to the history of a single policy Signed-off-by: Heiko W. Rupp --- .../cloud/policies/app/PolicyEngine.java | 2 + .../policies/app/rest/PolicyCrudService.java | 37 ++++++++++++++++--- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/redhat/cloud/policies/app/PolicyEngine.java b/src/main/java/com/redhat/cloud/policies/app/PolicyEngine.java index df533f2b..f3a9dabd 100644 --- a/src/main/java/com/redhat/cloud/policies/app/PolicyEngine.java +++ b/src/main/java/com/redhat/cloud/policies/app/PolicyEngine.java @@ -123,5 +123,7 @@ void disableTrigger(@PathParam("triggerId") UUID triggerId, @Consumes("application/json") String findLastTriggered(@QueryParam("triggerIds") String triggerIds, @QueryParam("thin") boolean thin, + @QueryParam("page") int page, + @QueryParam("per_page") int per_page, @HeaderParam("Hawkular-Tenant") String customerId); } diff --git a/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java b/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java index dc639326..26e21fd9 100644 --- a/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java +++ b/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java @@ -83,6 +83,8 @@ import org.eclipse.microprofile.rest.client.inject.RestClient; import org.hibernate.exception.ConstraintViolationException; +import static java.lang.Integer.min; + /** * @author hrupp */ @@ -128,7 +130,7 @@ public class PolicyCrudService { @Parameter( name = "limit", in = ParameterIn.QUERY, - description = "Number of items per page, if not specified uses 10. " + Pager.NO_LIMIT + " can be used to specify an unlimited page, when specified it ignores the offset" , + description = "Number of items per page, if not specified uses 50. " + Pager.NO_LIMIT + " can be used to specify an unlimited page, when specified it ignores the offset" , schema = @Schema(type = SchemaType.INTEGER) ), @Parameter( @@ -786,12 +788,26 @@ public Response getPolicy(@PathParam("id") UUID policyId) { @APIResponse(responseCode = "403", description = "Individual permissions missing to complete action") @APIResponse(responseCode = "404", description = "Policy not found") @APIResponse(responseCode = "500", description = "Retrieval of History failed") - @Parameter(name = "id", description = "UUID of the policy") + @Parameters({ + @Parameter( + name = "offset", + in = ParameterIn.QUERY, + description = "Page number, starts 0, if not specified uses 0.", + schema = @Schema(type = SchemaType.INTEGER) + ), + @Parameter( + name = "limit", + in = ParameterIn.QUERY, + description = "Number of items per page, if not specified uses 50. Maximum value is 200.", + schema = @Schema(type = SchemaType.INTEGER) + ), + @Parameter(name = "id", description = "UUID of the policy") + }) @GET @Path("/{id}/history/trigger") public Response getTriggerHistoryForPolicy(@PathParam("id") UUID policyId) { if (!user.canReadAll()) { - return Response.status(Response.Status.FORBIDDEN).entity(new Msg("Missing permissions to retrieve policies")).build(); + return Response.status(Response.Status.FORBIDDEN).entity(new Msg("Missing permissions to retrieve the policy history")).build(); } Policy policy = Policy.findById(user.getAccount(), policyId); @@ -800,8 +816,16 @@ public Response getTriggerHistoryForPolicy(@PathParam("id") UUID policyId) { if (policy==null) { builder = Response.status(Response.Status.NOT_FOUND); } else { + try { - String alerts = engine.findLastTriggered(policyId.toString(), false, user.getAccount()); + Pager pager = PagingUtils.extractPager(uriInfo); + + int limit = pager.getLimit(); + limit = min(limit,200); + int pageNum = pager.getOffset() / limit; + + String alerts = engine.findLastTriggered(policyId.toString(), false, pageNum, limit, + user.getAccount()); List items = new ArrayList<>(); DocumentContext jp = JsonPath.parse(alerts); @@ -816,8 +840,9 @@ public Response getTriggerHistoryForPolicy(@PathParam("id") UUID policyId) { } builder = Response.ok(items); } catch (Exception e) { - log.warning("Retrieval of history failed: " + e.getMessage()); - builder = Response.serverError(); + String msg = "Retrieval of history failed with: " + e.getMessage(); + log.warning(msg); + builder = Response.serverError().entity(msg); } } return builder.build(); From 73ad09bfe379a37564aba1741f4d966fb5e086d7 Mon Sep 17 00:00:00 2001 From: "Heiko W. Rupp" Date: Wed, 10 Jun 2020 16:57:38 +0200 Subject: [PATCH 2/5] Turn into paged list. Signed-off-by: Heiko W. Rupp --- .../cloud/policies/app/rest/PolicyCrudService.java | 14 +++++++++----- .../cloud/policies/app/rest/utils/PagingUtils.java | 12 ++++++++++-- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java b/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java index 26e21fd9..f8337f36 100644 --- a/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java +++ b/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java @@ -784,7 +784,9 @@ public Response getPolicy(@PathParam("id") UUID policyId) { @Operation(summary = "Retrieve the trigger history of a single policy") @APIResponse(responseCode = "200", description = "History could be retrieved", - content = @Content (schema = @Schema(type = SchemaType.ARRAY, implementation = HistoryItem.class))) + content = @Content (schema = @Schema(implementation = PagingUtils.PagedResponse.class)), + headers = @Header(name = "TotalCount", description = "Total number of items found", + schema = @Schema(type = SchemaType.INTEGER))) @APIResponse(responseCode = "403", description = "Individual permissions missing to complete action") @APIResponse(responseCode = "404", description = "Policy not found") @APIResponse(responseCode = "500", description = "Retrieval of History failed") @@ -810,9 +812,9 @@ public Response getTriggerHistoryForPolicy(@PathParam("id") UUID policyId) { return Response.status(Response.Status.FORBIDDEN).entity(new Msg("Missing permissions to retrieve the policy history")).build(); } - Policy policy = Policy.findById(user.getAccount(), policyId); + ResponseBuilder builder ; - ResponseBuilder builder ; + Policy policy = Policy.findById(user.getAccount(), policyId); if (policy==null) { builder = Response.status(Response.Status.NOT_FOUND); } else { @@ -821,7 +823,8 @@ public Response getTriggerHistoryForPolicy(@PathParam("id") UUID policyId) { Pager pager = PagingUtils.extractPager(uriInfo); int limit = pager.getLimit(); - limit = min(limit,200); + // We dont allow unlimited or values > 200 + limit = limit == Pager.NO_LIMIT ? 50 : min(limit,200); int pageNum = pager.getOffset() / limit; String alerts = engine.findLastTriggered(policyId.toString(), false, pageNum, limit, @@ -838,7 +841,8 @@ public Response getTriggerHistoryForPolicy(@PathParam("id") UUID policyId) { HistoryItem hi = new HistoryItem(ctime,insights_id,name); items.add(hi); } - builder = Response.ok(items); + Page itemsPage = new Page<>(items,pager,-1); // TODO -1 is wrong + builder = PagingUtils.responseBuilder(itemsPage); } catch (Exception e) { String msg = "Retrieval of history failed with: " + e.getMessage(); log.warning(msg); diff --git a/src/main/java/com/redhat/cloud/policies/app/rest/utils/PagingUtils.java b/src/main/java/com/redhat/cloud/policies/app/rest/utils/PagingUtils.java index cd996be5..824c7c37 100644 --- a/src/main/java/com/redhat/cloud/policies/app/rest/utils/PagingUtils.java +++ b/src/main/java/com/redhat/cloud/policies/app/rest/utils/PagingUtils.java @@ -169,12 +169,12 @@ public static ResponseBuilder responseBuilder(Page page) { */ public static class PagedResponse { - public Map meta = new HashMap<>(1); + public Meta meta; public Map links = new HashMap<>(3); public List data = new ArrayList<>(); public PagedResponse(Page page) { - meta.put("count", page.getTotalCount()); + meta = new Meta(page.getTotalCount()); data.addAll(page); @@ -200,4 +200,12 @@ public PagedResponse(Page page) { } } } + + public static class Meta { + public long count; + + public Meta(long count) { + this.count = count; + } + } } From 8b092f7d801ed9dc2d8b1cb17eee2cff9ce1ee41 Mon Sep 17 00:00:00 2001 From: "Heiko W. Rupp" Date: Mon, 15 Jun 2020 10:45:23 +0200 Subject: [PATCH 3/5] Fix test Signed-off-by: Heiko W. Rupp --- src/test/java/com/redhat/cloud/policies/app/RestApiTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/redhat/cloud/policies/app/RestApiTest.java b/src/test/java/com/redhat/cloud/policies/app/RestApiTest.java index d2d55d81..247e4a35 100644 --- a/src/test/java/com/redhat/cloud/policies/app/RestApiTest.java +++ b/src/test/java/com/redhat/cloud/policies/app/RestApiTest.java @@ -527,7 +527,7 @@ void getOnePolicyHistory() { .statusCode(200) .extract(); - List returnedBody = er.body().as(List.class); + List returnedBody = er.body().jsonPath().getList("data"); try { Assert.assertEquals(2, returnedBody.size()); Map map = (Map) returnedBody.get(0); From 1b12dc57914816f3a559736a41b2771199c23bb1 Mon Sep 17 00:00:00 2001 From: "Heiko W. Rupp" Date: Tue, 16 Jun 2020 18:18:30 +0200 Subject: [PATCH 4/5] Fetch total count from engine. Change openapi types to not erase the inner type Signed-off-by: Heiko W. Rupp --- .../cloud/policies/app/PolicyEngine.java | 3 +- .../policies/app/rest/PolicyCrudService.java | 40 +++++++++++++++---- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/redhat/cloud/policies/app/PolicyEngine.java b/src/main/java/com/redhat/cloud/policies/app/PolicyEngine.java index f3a9dabd..7161cfad 100644 --- a/src/main/java/com/redhat/cloud/policies/app/PolicyEngine.java +++ b/src/main/java/com/redhat/cloud/policies/app/PolicyEngine.java @@ -31,6 +31,7 @@ import javax.ws.rs.PathParam; import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; +import javax.ws.rs.core.Response; import com.redhat.cloud.policies.app.model.engine.Trigger; import org.eclipse.microprofile.rest.client.annotation.RegisterProvider; @@ -121,7 +122,7 @@ void disableTrigger(@PathParam("triggerId") UUID triggerId, @GET @Consumes("application/json") - String findLastTriggered(@QueryParam("triggerIds") String triggerIds, + Response findLastTriggered(@QueryParam("triggerIds") String triggerIds, @QueryParam("thin") boolean thin, @QueryParam("page") int page, @QueryParam("per_page") int per_page, diff --git a/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java b/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java index f8337f36..9b745232 100644 --- a/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java +++ b/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java @@ -95,7 +95,7 @@ @RequestScoped public class PolicyCrudService { - private Logger log = Logger.getLogger(this.getClass().getSimpleName()); + private final Logger log = Logger.getLogger(this.getClass().getSimpleName()); @Inject @RestClient @@ -117,6 +117,16 @@ public class PolicyCrudService { @Inject Validator validator; + // workaround for returning generic types: https://github.com/swagger-api/swagger-core/issues/498#issuecomment-74510379 + // This class is used only for swagger return type + private static class PagedResponseOfPolicy extends PagingUtils.PagedResponse { + @SuppressWarnings("unused") + private List data; + private PagedResponseOfPolicy(Page page) { + super(page); + } + } + @Operation(summary = "Return all policies for a given account") @GET @Path("/") @@ -213,7 +223,7 @@ public class PolicyCrudService { @APIResponse(responseCode = "404", description = "No policies found for customer") @APIResponse(responseCode = "403", description = "Individual permissions missing to complete action") @APIResponse(responseCode = "200", description = "Policies found", content = - @Content(schema = @Schema(implementation = PagingUtils.PagedResponse.class)), + @Content(schema = @Schema(implementation = PagedResponseOfPolicy.class)), headers = @Header(name = "TotalCount", description = "Total number of items found", schema = @Schema(type = SchemaType.INTEGER))) public Response getPoliciesForCustomer() { @@ -731,7 +741,7 @@ public Response validateName(@NotNull JsonString policyName, @QueryParam("id") U if (result.size() > 0) { String error = String.join( ";", - result.stream().map(policyConstraintViolation -> policyConstraintViolation.getMessage()).collect(Collectors.toSet()) + result.stream().map(ConstraintViolation::getMessage).collect(Collectors.toSet()) ); return Response.status(400).entity(new Msg(error)).build(); } @@ -782,9 +792,19 @@ public Response getPolicy(@PathParam("id") UUID policyId) { return builder.build(); } + // workaround for returning generic types: https://github.com/swagger-api/swagger-core/issues/498#issuecomment-74510379 + // This class is used only for swagger return type + private static class PagedResponseOfHistoryItem extends PagingUtils.PagedResponse { + @SuppressWarnings("unused") + private List data; + private PagedResponseOfHistoryItem(Page page) { + super(page); + } + } + @Operation(summary = "Retrieve the trigger history of a single policy") @APIResponse(responseCode = "200", description = "History could be retrieved", - content = @Content (schema = @Schema(implementation = PagingUtils.PagedResponse.class)), + content = @Content (schema = @Schema(implementation = PagedResponseOfHistoryItem.class)), headers = @Header(name = "TotalCount", description = "Total number of items found", schema = @Schema(type = SchemaType.INTEGER))) @APIResponse(responseCode = "403", description = "Individual permissions missing to complete action") @@ -827,12 +847,16 @@ public Response getTriggerHistoryForPolicy(@PathParam("id") UUID policyId) { limit = limit == Pager.NO_LIMIT ? 50 : min(limit,200); int pageNum = pager.getOffset() / limit; - String alerts = engine.findLastTriggered(policyId.toString(), false, pageNum, limit, + Response response = engine.findLastTriggered(policyId.toString(), false, pageNum, limit, user.getAccount()); + String countHeader = response.getHeaderString("X-Total-Count"); + long totalCount = Long.parseLong(countHeader); + + String alerts = response.readEntity(String.class); List items = new ArrayList<>(); - DocumentContext jp = JsonPath.parse(alerts); - List> list = jp.read("$.[*].evalSets..value"); + DocumentContext jp = JsonPath.parse(alerts); + List> list = jp.read("$.[*].evalSets..value"); for (Map value : list) { long ctime = (long) value.get("ctime"); Map tmp = (Map) value.get("tags"); @@ -841,7 +865,7 @@ public Response getTriggerHistoryForPolicy(@PathParam("id") UUID policyId) { HistoryItem hi = new HistoryItem(ctime,insights_id,name); items.add(hi); } - Page itemsPage = new Page<>(items,pager,-1); // TODO -1 is wrong + Page itemsPage = new Page<>(items,pager,totalCount); builder = PagingUtils.responseBuilder(itemsPage); } catch (Exception e) { String msg = "Retrieval of history failed with: " + e.getMessage(); From 6de5a8f5020f87046c3eb38fbbc025071ba8f614 Mon Sep 17 00:00:00 2001 From: "Heiko W. Rupp" Date: Thu, 18 Jun 2020 10:25:08 +0200 Subject: [PATCH 5/5] Adjust tests. Signed-off-by: Heiko W. Rupp --- .../redhat/cloud/policies/app/rest/PolicyCrudService.java | 4 ++-- src/test/java/com/redhat/cloud/policies/app/RestApiTest.java | 5 ++++- .../com/redhat/cloud/policies/app/TestLifecycleManager.java | 1 + 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java b/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java index 9b745232..496a53b3 100644 --- a/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java +++ b/src/main/java/com/redhat/cloud/policies/app/rest/PolicyCrudService.java @@ -860,9 +860,9 @@ public Response getTriggerHistoryForPolicy(@PathParam("id") UUID policyId) { for (Map value : list) { long ctime = (long) value.get("ctime"); Map tmp = (Map) value.get("tags"); - String insights_id = (String) tmp.get("inventory_id"); + String inventory_id = (String) tmp.get("inventory_id"); String name = (String) tmp.get("display_name"); - HistoryItem hi = new HistoryItem(ctime,insights_id,name); + HistoryItem hi = new HistoryItem(ctime,inventory_id,name); items.add(hi); } Page itemsPage = new Page<>(items,pager,totalCount); diff --git a/src/test/java/com/redhat/cloud/policies/app/RestApiTest.java b/src/test/java/com/redhat/cloud/policies/app/RestApiTest.java index 247e4a35..eae12514 100644 --- a/src/test/java/com/redhat/cloud/policies/app/RestApiTest.java +++ b/src/test/java/com/redhat/cloud/policies/app/RestApiTest.java @@ -527,7 +527,10 @@ void getOnePolicyHistory() { .statusCode(200) .extract(); - List returnedBody = er.body().jsonPath().getList("data"); + JsonPath jsonPath = er.body().jsonPath(); + int totalCount = jsonPath.getInt("meta.count"); + Assert.assertEquals(2,totalCount); + List returnedBody = jsonPath.getList("data"); try { Assert.assertEquals(2, returnedBody.size()); Map map = (Map) returnedBody.get(0); diff --git a/src/test/java/com/redhat/cloud/policies/app/TestLifecycleManager.java b/src/test/java/com/redhat/cloud/policies/app/TestLifecycleManager.java index 875eeac7..c6dd6125 100644 --- a/src/test/java/com/redhat/cloud/policies/app/TestLifecycleManager.java +++ b/src/test/java/com/redhat/cloud/policies/app/TestLifecycleManager.java @@ -183,6 +183,7 @@ private void mockEngine() { .respond(response() .withStatusCode(200) .withHeader("Content-Type", "application/json") + .withHeader("X-Total-Count","2") .withBody(JsonBody.json(alertsHistory)) );