fix: validation results paging [DHIS2-19173] (2.41) (#20232)

* fix: validation results paging [DHIS2-19173] (#20229)

* fix: undo OpenApi.Description

* fix: imported class name

* fix: import

Author: jbee

dhis-2/dhis-api/src/main/java/org/hisp/dhis/validation/comparator/ValidationResultQuery.java

@@ -27,19 +27,25 @@
  */
 package org.hisp.dhis.validation.comparator;
 
-import com.google.common.base.MoreObjects;
 import java.util.Date;
 import java.util.List;
-import org.apache.commons.lang3.BooleanUtils;
+import lombok.Getter;
+import lombok.Setter;
+import lombok.ToString;
 import org.hisp.dhis.common.OpenApi;
 import org.hisp.dhis.common.Pager;
 import org.hisp.dhis.common.PagerUtils;
+import org.hisp.dhis.common.UID;
+import org.hisp.dhis.organisationunit.OrganisationUnit;
+import org.hisp.dhis.validation.ValidationRule;
 
 /**
  * @author Stian Sandvold
  */
+@Setter
+@Getter
+@ToString
 public class ValidationResultQuery {
-  public static final ValidationResultQuery EMPTY = new ValidationResultQuery();
 
   private Boolean skipPaging;
 
@@ -49,117 +55,32 @@ public class ValidationResultQuery {
 
   private int pageSize = Pager.DEFAULT_PAGE_SIZE;
 
-  private long total;
-
   /**
-   * Optional list of validation rule uids to filter. If empty the list is not restricting the
-   * query.
+   * Optional list of validation rule UIDs to filter. If empty the list is not restricting the query
    */
+  @OpenApi.Property({UID.class, ValidationRule.class})
   private List<String> vr;
 
   /**
-   * Optional list of organisation unit uids to filter. If empty the list is not restricting the
-   * query.
+   * Optional list of organisation unit UIDs to filter. If empty the list is not restricting the
+   * query
    */
+  @OpenApi.Property({UID.class, OrganisationUnit.class})
   private List<String> ou;
 
   /**
-   * Optional list of ISO-Date expressions to filter. If empty the list is not restricting the
-   * query.
+   * Optional list of ISO-Date expressions to filter. If empty the list is not restricting the query
    */
   private List<String> pe;
 
-  /** Optional filter to select only results that have been created on or after the given date. */
+  /** Optional filter to select only results that have been created on or after the given date */
   private Date createdDate;
 
+  private List<String> fields;
+
   public ValidationResultQuery() {}
 
   public boolean isSkipPaging() {
     return PagerUtils.isSkipPaging(skipPaging, paging);
   }
-
-  public void setSkipPaging(Boolean skipPaging) {
-    this.skipPaging = skipPaging;
-  }
-
-  public boolean isPaging() {
-    return BooleanUtils.toBoolean(paging);
-  }
-
-  public void setPaging(Boolean paging) {
-    this.paging = paging;
-  }
-
-  public int getPage() {
-    return page;
-  }
-
-  public void setPage(int page) {
-    this.page = page;
-  }
-
-  public int getPageSize() {
-    return pageSize;
-  }
-
-  public void setPageSize(int pageSize) {
-    this.pageSize = pageSize;
-  }
-
-  public long getTotal() {
-    return total;
-  }
-
-  public void setTotal(long total) {
-    this.total = total;
-  }
-
-  @OpenApi.Ignore
-  public Pager getPager() {
-    return PagerUtils.isSkipPaging(skipPaging, paging) ? null : new Pager(page, total, pageSize);
-  }
-
-  public List<String> getVr() {
-    return vr;
-  }
-
-  public void setVr(List<String> vr) {
-    this.vr = vr;
-  }
-
-  public List<String> getOu() {
-    return ou;
-  }
-
-  public void setOu(List<String> ou) {
-    this.ou = ou;
-  }
-
-  public List<String> getPe() {
-    return pe;
-  }
-
-  public void setPe(List<String> pe) {
-    this.pe = pe;
-  }
-
-  public Date getCreatedDate() {
-    return createdDate;
-  }
-
-  public void setCreatedDate(Date createdDate) {
-    this.createdDate = createdDate;
-  }
-
-  @Override
-  public String toString() {
-    return MoreObjects.toStringHelper(this)
-        .add("page", page)
-        .add("pageSize", pageSize)
-        .add("total", total)
-        .add("ou", ou)
-        .add("vr", vr)
-        .add("pe", pe)
-        .toString();
-  }
 }

dhis-2/dhis-services/dhis-service-validation/src/main/java/org/hisp/dhis/validation/hibernate/HibernateValidationResultStore.java

@@ -45,7 +45,6 @@
 import org.hisp.dhis.category.CategoryOptionGroupSet;
 import org.hisp.dhis.common.IdentifiableObject;
 import org.hisp.dhis.common.IdentifiableObjectUtils;
-import org.hisp.dhis.common.Pager;
 import org.hisp.dhis.commons.util.SqlHelper;
 import org.hisp.dhis.hibernate.HibernateGenericStore;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
@@ -169,9 +168,8 @@ public List<ValidationResult> query(ValidationResultQuery query) {
     addQueryParameters(query, hibernateQuery);
 
     if (!query.isSkipPaging()) {
-      Pager pager = query.getPager();
-      hibernateQuery.setFirstResult(pager.getOffset());
-      hibernateQuery.setMaxResults(pager.getPageSize());
+      hibernateQuery.setFirstResult((query.getPage() - 1) * query.getPageSize());
+      hibernateQuery.setMaxResults(query.getPageSize());
     }
 
     return hibernateQuery.getResultList();

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/AbstractFullReadOnlyController.java

@@ -170,7 +170,7 @@ protected void forceFiltering(final WebOptions webOptions, final List<String> fi
 
   @Value
   @OpenApi.Shared(value = false)
-  protected static class ObjectListResponse {
+  public static class ObjectListResponse {
     @OpenApi.Property Pager pager;
 
     @OpenApi.Property(name = "path$", value = OpenApi.EntityType[].class)

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/validation/ValidationResultController.java

@@ -27,27 +27,26 @@
  */
 package org.hisp.dhis.webapi.controller.validation;
 
-import static org.hisp.dhis.dxf2.webmessage.WebMessageUtils.notFound;
 import static org.hisp.dhis.webapi.utils.ContextUtils.setNoStore;
 
-import com.google.common.collect.Lists;
 import java.util.List;
 import javax.servlet.http.HttpServletResponse;
+import lombok.RequiredArgsConstructor;
 import org.hisp.dhis.common.DhisApiVersion;
 import org.hisp.dhis.common.OpenApi;
-import org.hisp.dhis.dxf2.webmessage.WebMessageException;
+import org.hisp.dhis.common.Pager;
+import org.hisp.dhis.feedback.NotFoundException;
 import org.hisp.dhis.fieldfilter.FieldFilterParams;
 import org.hisp.dhis.fieldfilter.FieldFilterService;
-import org.hisp.dhis.fieldfiltering.Preset;
 import org.hisp.dhis.node.NodeUtils;
 import org.hisp.dhis.node.types.RootNode;
 import org.hisp.dhis.schema.descriptors.ValidationResultSchemaDescriptor;
 import org.hisp.dhis.validation.ValidationResult;
 import org.hisp.dhis.validation.ValidationResultService;
 import org.hisp.dhis.validation.ValidationResultsDeletionRequest;
 import org.hisp.dhis.validation.comparator.ValidationResultQuery;
+import org.hisp.dhis.webapi.controller.AbstractFullReadOnlyController;
 import org.hisp.dhis.webapi.mvc.annotation.ApiVersion;
-import org.hisp.dhis.webapi.service.ContextService;
 import org.springframework.http.HttpStatus;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.web.bind.annotation.DeleteMapping;
@@ -62,41 +61,34 @@
  * @author Stian Sandvold
  */
 @OpenApi.Tags("data")
+@OpenApi.EntityType(ValidationResult.class)
 @RestController
 @RequestMapping(value = ValidationResultSchemaDescriptor.API_ENDPOINT)
 @ApiVersion({DhisApiVersion.ALL, DhisApiVersion.DEFAULT})
+@RequiredArgsConstructor
 public class ValidationResultController {
-  private final FieldFilterService fieldFilterService;
 
+  private final FieldFilterService fieldFilterService;
   private final ValidationResultService validationResultService;
 
-  private final ContextService contextService;
-
-  public ValidationResultController(
-      FieldFilterService fieldFilterService,
-      ValidationResultService validationResultService,
-      ContextService contextService) {
-    this.fieldFilterService = fieldFilterService;
-    this.validationResultService = validationResultService;
-    this.contextService = contextService;
-  }
-
   @GetMapping
+  @OpenApi.Response(AbstractFullReadOnlyController.ObjectListResponse.class)
   public @ResponseBody RootNode getObjectList(
       ValidationResultQuery query, HttpServletResponse response) {
-    List<String> fields = Lists.newArrayList(contextService.getParameterValues("fields"));
+    List<String> fields = query.getFields();
 
-    if (fields.isEmpty()) {
-      fields.addAll(Preset.ALL.getFields());
+    if (fields == null || fields.isEmpty()) {
+      fields = List.of("*");
     }
 
     List<ValidationResult> validationResults = validationResultService.getValidationResults(query);
 
     RootNode rootNode = NodeUtils.createMetadata();
 
     if (!query.isSkipPaging()) {
-      query.setTotal(validationResultService.countValidationResults(query));
-      rootNode.addChild(NodeUtils.createPager(query.getPager()));
+      long total = validationResultService.countValidationResults(query);
+      rootNode.addChild(
+          NodeUtils.createPager(new Pager(query.getPage(), total, query.getPageSize())));
     }
 
     rootNode.addChild(
@@ -108,18 +100,18 @@ public ValidationResultController(
   }
 
   @GetMapping(value = "/{id}")
-  public @ResponseBody ValidationResult getObject(@PathVariable int id) throws WebMessageException {
+  public @ResponseBody ValidationResult getObject(@PathVariable int id) throws NotFoundException {
     ValidationResult result = validationResultService.getById(id);
-    checkFound(id, result);
+    if (result == null) throw new NotFoundException(ValidationResult.class, "" + id);
     return result;
   }
 
   @PreAuthorize("hasRole('F_PERFORM_MAINTENANCE')")
   @DeleteMapping(value = "/{id}")
   @ResponseStatus(value = HttpStatus.NO_CONTENT)
-  public void delete(@PathVariable int id) throws WebMessageException {
+  public void delete(@PathVariable int id) throws NotFoundException {
     ValidationResult result = validationResultService.getById(id);
-    checkFound(id, result);
+    if (result == null) throw new NotFoundException(ValidationResult.class, "" + id);
     validationResultService.deleteValidationResult(result);
   }
 
@@ -129,10 +121,4 @@ public void delete(@PathVariable int id) throws WebMessageException {
   public void deleteValidationResults(ValidationResultsDeletionRequest request) {
     validationResultService.deleteValidationResults(request);
   }
-
-  private void checkFound(int id, ValidationResult result) throws WebMessageException {
-    if (result == null) {
-      throw new WebMessageException(notFound("Validation result with id " + id + " was not found"));
-    }
-  }
 }