SWATCH-3177 Fix invalid remittance calculation when an amendment is applied

src/main/resources/liquibase/202412091327-drop-hardware-measurement-type-from-billable-usage-remittance.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<databaseChangeLog
+        xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
+        http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.20.xsd">
+  <changeSet id="202412091327-01" author="mstead">
+    <comment>Drop the hardware_measurement_type column from billable_usage_remittance table.</comment>
+    <dropColumn tableName="billable_usage_remittance" columnName="hardware_measurement_type" />
+    <rollback>
+      <addColumn tableName="billable_usage_remittance">
+        <column name="hardware_measurement_type" type="varchar(32)" />
+      </addColumn>
+    </rollback>
+  </changeSet>
+</databaseChangeLog>

src/main/resources/liquibase/changelog.xml

@@ -177,5 +177,6 @@
     <include file="/liquibase/202410241408-clear-retryable.xml"/>
     <include file="/liquibase/202412041537-primary-keys-on-all-tables.xml"/>
     <include file="/liquibase/202501081035-remove-backup-tables.xml"/>
+    <include file="/liquibase/202412091327-drop-hardware-measurement-type-from-billable-usage-remittance.xml"/>
 </databaseChangeLog>
 <!-- vim: set expandtab sts=4 sw=4 ai: -->

swatch-billable-usage/src/main/java/com/redhat/swatch/billable/usage/data/BillableUsageRemittanceEntity.java

@@ -85,9 +85,6 @@ public class BillableUsageRemittanceEntity implements Serializable {
   @Column(name = "tally_id")
   private UUID tallyId;
 
-  @Column(name = "hardware_measurement_type")
-  private String hardwareMeasurementType;
-
   @Column(name = "status")
   private RemittanceStatus status;
 

swatch-billable-usage/src/main/java/com/redhat/swatch/billable/usage/data/BillableUsageRemittanceFilter.java

@@ -46,14 +46,13 @@ public class BillableUsageRemittanceFilter {
   private String metricId;
   private String billingProvider;
   private String billingAccountId;
-  private String hardwareMeasurementType;
   private OffsetDateTime beginning;
   private OffsetDateTime ending;
   private String accumulationPeriod;
   private boolean excludeFailures;
   private UUID tallyId;
 
-  public static BillableUsageRemittanceFilter fromUsage(BillableUsage usage) {
+  public static BillableUsageRemittanceFilter totalRemittedFilter(BillableUsage usage) {
     return BillableUsageRemittanceFilter.builder()
         .orgId(usage.getOrgId())
         .billingAccountId(usage.getBillingAccountId())
@@ -63,7 +62,7 @@ public static BillableUsageRemittanceFilter fromUsage(BillableUsage usage) {
         .productId(usage.getProductId())
         .sla(usage.getSla().value())
         .usage(usage.getUsage().value())
-        .hardwareMeasurementType(usage.getHardwareMeasurementType())
+        .excludeFailures(true)
         .build();
   }
 }

swatch-billable-usage/src/main/java/com/redhat/swatch/billable/usage/data/BillableUsageRemittanceRepository.java

@@ -64,7 +64,6 @@ public List<RemittanceSummaryProjection> getRemittanceSummaries(
           root.get(BillableUsageRemittanceEntity_.METRIC_ID),
           root.get(BillableUsageRemittanceEntity_.ORG_ID),
           root.get(BillableUsageRemittanceEntity_.PRODUCT_ID),
-          root.get(BillableUsageRemittanceEntity_.HARDWARE_MEASUREMENT_TYPE),
           root.get(BillableUsageRemittanceEntity_.STATUS),
           root.get(BillableUsageRemittanceEntity_.ERROR_CODE));
     }
@@ -81,7 +80,6 @@ public List<RemittanceSummaryProjection> getRemittanceSummaries(
             root.get(BillableUsageRemittanceEntity_.BILLING_PROVIDER),
             root.get(BillableUsageRemittanceEntity_.BILLING_ACCOUNT_ID),
             root.get(BillableUsageRemittanceEntity_.METRIC_ID),
-            root.get(BillableUsageRemittanceEntity_.HARDWARE_MEASUREMENT_TYPE),
             root.get(BillableUsageRemittanceEntity_.STATUS),
             root.get(BillableUsageRemittanceEntity_.ERROR_CODE)));
     return entityManager.createQuery(query).getResultList();
@@ -171,10 +169,6 @@ private Specification<BillableUsageRemittanceEntity> buildSearchSpecification(
     if (Objects.nonNull(filter.getSla())) {
       searchCriteria = searchCriteria.and(matchingSla(filter.getSla()));
     }
-    if (Objects.nonNull(filter.getHardwareMeasurementType())) {
-      searchCriteria =
-          searchCriteria.and(matchingHardwareMeasurementType(filter.getHardwareMeasurementType()));
-    }
     if (filter.isExcludeFailures()) {
       searchCriteria = searchCriteria.and(excludesFailures());
     }
@@ -193,14 +187,6 @@ private Specification<BillableUsageRemittanceEntity> excludesFailures() {
                 root.get(BillableUsageRemittanceEntity_.status), RemittanceStatus.FAILED));
   }
 
-  private Specification<BillableUsageRemittanceEntity> matchingHardwareMeasurementType(
-      String hardwareMeasurementType) {
-    return (root, query, builder) ->
-        builder.equal(
-            root.get(BillableUsageRemittanceEntity_.hardwareMeasurementType),
-            hardwareMeasurementType);
-  }
-
   private static Specification<BillableUsageRemittanceEntity> matchingProductId(String productId) {
     return (root, query, builder) ->
         builder.equal(root.get(BillableUsageRemittanceEntity_.productId), productId);

swatch-billable-usage/src/main/java/com/redhat/swatch/billable/usage/data/RemittanceSummaryProjection.java

@@ -41,7 +41,6 @@ public class RemittanceSummaryProjection {
   private String billingProvider;
   private String billingAccountId;
   private String metricId;
-  private String hardwareMeasurementType;
   private RemittanceStatus status;
   private RemittanceErrorCode errorCode;
 }

swatch-billable-usage/src/main/java/com/redhat/swatch/billable/usage/services/BillableUsageMapper.java

@@ -112,7 +112,6 @@ private BillableUsage toBillableUsage(
         .withBillingAccountId(snapshot.getBillingAccountId())
         .withMetricId(measurement.getMetricId())
         .withValue(measurement.getValue())
-        .withHardwareMeasurementType(measurement.getHardwareMeasurementType())
         .withCurrentTotal(measurement.getCurrentTotal());
   }
 

swatch-billable-usage/src/main/java/com/redhat/swatch/billable/usage/services/BillableUsageService.java

@@ -170,8 +170,7 @@ private ContractCoverage getContractCoverage(BillableUsage usage)
   }
 
   protected double getTotalRemitted(BillableUsage billableUsage) {
-    var filter = BillableUsageRemittanceFilter.fromUsage(billableUsage);
-    filter.setExcludeFailures(true);
+    var filter = BillableUsageRemittanceFilter.totalRemittedFilter(billableUsage);
     return billableUsageRemittanceRepository.getRemittanceSummaries(filter).stream()
         .map(RemittanceSummaryProjection::getTotalRemittedPendingValue)
         .reduce(Double::sum)
@@ -236,7 +235,6 @@ private void createRemittance(
             .usage(usage.getUsage().value())
             .remittancePendingDate(clock.now())
             .tallyId(usage.getTallyId())
-            .hardwareMeasurementType(usage.getHardwareMeasurementType())
             .status(
                 contractCoverage.isGratis() ? RemittanceStatus.GRATIS : RemittanceStatus.PENDING)
             .build();

swatch-billable-usage/src/test/java/com/redhat/swatch/billable/usage/data/BillableUsageRemittanceRepositoryTest.java

@@ -135,7 +135,6 @@ private BillableUsageRemittanceEntity remittance(
         .accumulationPeriod(AccumulationPeriodFormatter.toMonthId(remittanceDate))
         .remittancePendingDate(remittanceDate)
         .remittedPendingValue(value)
-        .hardwareMeasurementType("AWS")
         .build();
   }
 
@@ -335,7 +334,6 @@ void getMonthlySummary() {
             .totalRemittedPendingValue(24.0)
             .sla(remittance1.getSla())
             .usage(remittance1.getUsage())
-            .hardwareMeasurementType("AWS")
             .build();
 
     var expectedSummary2 =
@@ -350,7 +348,6 @@ void getMonthlySummary() {
             .totalRemittedPendingValue(15.0)
             .sla(remittance3.getSla())
             .usage(remittance3.getUsage())
-            .hardwareMeasurementType("AWS")
             .build();
 
     List<RemittanceSummaryProjection> results = repository.getRemittanceSummaries(filter1);
@@ -370,7 +367,24 @@ void testRemittanceFilterUsedByBillableUsageService() {
     String billingProvider = "aws";
     String billingAccountId = "ba123456789";
     String remittancePendingDateStr = "2024-06-19T16:52:17.526219+00:00";
-    String hardwareMeasurementType = "AWS";
+
+    var billableUsageNoExpected =
+        BillableUsageRemittanceEntity.builder()
+            .orgId(orgId)
+            .productId("another_product")
+            .metricId(metricId)
+            .accumulationPeriod("2024-06")
+            .sla(sla)
+            .usage(usage)
+            .billingProvider(billingProvider)
+            .billingAccountId(billingAccountId)
+            .remittedPendingValue(1.0)
+            .remittancePendingDate(
+                OffsetDateTime.parse(
+                    remittancePendingDateStr, DateTimeFormatter.ISO_OFFSET_DATE_TIME))
+            .tallyId(UUID.randomUUID())
+            .status(RemittanceStatus.PENDING)
+            .build();
 
     var billableUsageRemittanceFromCost =
         BillableUsageRemittanceEntity.builder()
@@ -387,7 +401,6 @@ void testRemittanceFilterUsedByBillableUsageService() {
                 OffsetDateTime.parse(
                     remittancePendingDateStr, DateTimeFormatter.ISO_OFFSET_DATE_TIME))
             .tallyId(UUID.randomUUID())
-            .hardwareMeasurementType(hardwareMeasurementType)
             .status(RemittanceStatus.PENDING)
             .build();
 
@@ -407,11 +420,13 @@ void testRemittanceFilterUsedByBillableUsageService() {
                 OffsetDateTime.parse(
                     remittancePendingDateStr, DateTimeFormatter.ISO_OFFSET_DATE_TIME))
             .tallyId(UUID.randomUUID())
-            .hardwareMeasurementType("PHYSICAL")
-            .status(RemittanceStatus.PENDING)
+            .status(RemittanceStatus.SUCCEEDED)
             .build();
     repository.persist(
-        List.of(billableUsageRemittanceFromCost, billableUsageRemittanceFromRhelemeter));
+        List.of(
+            billableUsageRemittanceFromCost,
+            billableUsageRemittanceFromRhelemeter,
+            billableUsageNoExpected));
 
     BillableUsage incomingUsage = new BillableUsage();
     incomingUsage
@@ -424,15 +439,14 @@ void testRemittanceFilterUsedByBillableUsageService() {
         .withMetricId(metricId)
         .withProductId(productTag)
         .withUsage(BillableUsage.Usage.PRODUCTION)
-        .withSla(BillableUsage.Sla.PREMIUM)
-        .withHardwareMeasurementType("PHYSICAL");
+        .withSla(BillableUsage.Sla.PREMIUM);
 
     // Build the filter the same way that we do in the billable usage in the service.
-    var filter = BillableUsageRemittanceFilter.fromUsage(incomingUsage);
+    var filter = BillableUsageRemittanceFilter.totalRemittedFilter(incomingUsage);
 
     List<RemittanceSummaryProjection> remittanceSummaries =
         repository.getRemittanceSummaries(filter);
-    assertEquals(1, remittanceSummaries.size());
+    assertEquals(2, remittanceSummaries.size());
 
     var expectedSummary =
         RemittanceSummaryProjection.builder()
@@ -448,12 +462,28 @@ void testRemittanceFilterUsedByBillableUsageService() {
             .totalRemittedPendingValue(expectedRemittedPendingValue)
             .sla(billableUsageRemittanceFromRhelemeter.getSla())
             .usage(billableUsageRemittanceFromRhelemeter.getUsage())
-            .hardwareMeasurementType(
-                billableUsageRemittanceFromRhelemeter.getHardwareMeasurementType())
-            .status(RemittanceStatus.PENDING)
+            .status(billableUsageRemittanceFromRhelemeter.getStatus())
+            .build();
+
+    assertTrue(remittanceSummaries.contains(expectedSummary));
+
+    var expectedSummary2 =
+        RemittanceSummaryProjection.builder()
+            .accumulationPeriod(
+                getAccumulationPeriod(billableUsageRemittanceFromCost.getRemittancePendingDate()))
+            .billingAccountId(billableUsageRemittanceFromCost.getBillingAccountId())
+            .billingProvider(billableUsageRemittanceFromCost.getBillingProvider())
+            .orgId(billableUsageRemittanceFromCost.getOrgId())
+            .productId(billableUsageRemittanceFromCost.getProductId())
+            .remittancePendingDate(billableUsageRemittanceFromCost.getRemittancePendingDate())
+            .metricId(billableUsageRemittanceFromCost.getMetricId())
+            .totalRemittedPendingValue(billableUsageRemittanceFromCost.getRemittedPendingValue())
+            .sla(billableUsageRemittanceFromCost.getSla())
+            .usage(billableUsageRemittanceFromCost.getUsage())
+            .status(billableUsageRemittanceFromCost.getStatus())
             .build();
 
-    assertEquals(expectedSummary, remittanceSummaries.get(0));
+    assertTrue(remittanceSummaries.contains(expectedSummary2));
   }
 
   @Test

swatch-billable-usage/src/test/java/com/redhat/swatch/billable/usage/services/BillableUsageMapperTest.java

@@ -194,7 +194,6 @@ void shouldProduceBillableUsageWhenOrgIdPresent() {
             .withBillingAccountId(expectedBillingAccountId)
             .withMetricId(expectedMetricId)
             .withValue(42.0)
-            .withHardwareMeasurementType("PHYSICAL")
             .withCurrentTotal(expectedCurrentTotal);
 
     var summary =

swatch-billable-usage/src/test/java/com/redhat/swatch/billable/usage/services/BillableUsageServiceTest.java

@@ -593,7 +593,6 @@ void givenExistingRemittanceForUsage(
             .usage(usage.getUsage().value())
             .remittancePendingDate(remittancePendingDate)
             .tallyId(usage.getTallyId())
-            .hardwareMeasurementType(usage.getHardwareMeasurementType())
             .status(status)
             .build();
     // Remitted value should be set to usages metric_value rather than billing_value

swatch-core/schemas/billable_usage.yaml

@@ -79,9 +79,6 @@ properties:
   vendor_product_code:
     description: Field used to specify a unique contract
     type: string
-  hardware_measurement_type:
-    type: string
-    format: string
   currentTotal:
     description: Sum of all measurements between the start of the month to the snapshot date.
     type: number

swatch-core/src/main/java/org/candlepin/subscriptions/db/TallySnapshotRepository.java

@@ -20,6 +20,10 @@
  */
 package org.candlepin.subscriptions.db;
 
+import static org.hibernate.jpa.AvailableHints.HINT_FETCH_SIZE;
+import static org.hibernate.jpa.AvailableHints.HINT_READ_ONLY;
+
+import jakarta.persistence.QueryHint;
 import java.time.OffsetDateTime;
 import java.util.Collection;
 import java.util.UUID;
@@ -35,6 +39,7 @@
 import org.springframework.data.jpa.repository.JpaRepository;
 import org.springframework.data.jpa.repository.Modifying;
 import org.springframework.data.jpa.repository.Query;
+import org.springframework.data.jpa.repository.QueryHints;
 import org.springframework.data.repository.query.Param;
 import org.springframework.transaction.annotation.Propagation;
 import org.springframework.transaction.annotation.Transactional;
@@ -153,19 +158,44 @@ Stream<TallySnapshot> findByOrgIdAndProductIdInAndGranularityAndSnapshotDateBetw
 
   void deleteByOrgId(String orgId);
 
-  @SuppressWarnings("java:S107") // repository method has a lot of params, deal with it
+  @SuppressWarnings("java:S107")
+  @QueryHints(
+      value = {
+        @QueryHint(name = HINT_FETCH_SIZE, value = "1024"),
+        @QueryHint(name = HINT_READ_ONLY, value = "true")
+      })
   @Query(
-      "select coalesce(sum(VALUE(m)), 0.0) from TallySnapshot s "
-          + "left join s.tallyMeasurements m on key(m) = :measurementKey "
-          + "where s.orgId = :orgId and "
-          + "s.productId = :productId and "
-          + "s.granularity = :granularity and "
-          + "s.serviceLevel = :serviceLevel and "
-          + "s.usage = :usage and "
-          + "s.billingProvider = :billingProvider and "
-          + "s.billingAccountId = :billingAcctId and "
-          + "s.snapshotDate >= :beginning and s.snapshotDate <= :ending")
+      value =
+          """
+      select coalesce(sum(m.value), 0.0) from tally_snapshots s
+      left join tally_measurements m on m.snapshot_id=s.id
+      where s.org_id = :orgId and
+        s.product_id = :productId and
+        s.granularity = :granularity and
+        s.sla = :serviceLevel and
+        s.usage = :usage and
+        s.billing_provider = :billingProvider and
+        s.billing_account_id = :billingAcctId and
+        s.snapshot_date >= :beginning and s.snapshot_date <= :ending and
+        m.measurement_type != 'TOTAL' and m.metric_id = :metricId
+      """,
+      nativeQuery = true)
   Double sumMeasurementValueForPeriod(
+      @Param("orgId") String orgId,
+      @Param("productId") String productId,
+      @Param("granularity") String granularity,
+      @Param("serviceLevel") String serviceLevel,
+      @Param("usage") String usage,
+      @Param("billingProvider") String billingProvider,
+      @Param("billingAcctId") String billingAccountId,
+      @Param("beginning") OffsetDateTime beginning,
+      @Param("ending") OffsetDateTime ending,
+      @Param("metricId") String metricId);
+
+  // Provided to allow passing enums instead of strings. Native queries need the String values
+  // of enums, so this is a common place to do the required name vs value conversion.
+  @SuppressWarnings("java:S107")
+  default Double sumMeasurementValueForPeriod(
       @Param("orgId") String orgId,
       @Param("productId") String productId,
       @Param("granularity") Granularity granularity,
@@ -175,7 +205,19 @@ Double sumMeasurementValueForPeriod(
       @Param("billingAcctId") String billingAccountId,
       @Param("beginning") OffsetDateTime beginning,
       @Param("ending") OffsetDateTime ending,
-      @Param("measurementKey") TallyMeasurementKey measurementKey);
+      @Param("measurementKey") TallyMeasurementKey measurementKey) {
+    return this.sumMeasurementValueForPeriod(
+        orgId,
+        productId,
+        granularity.name(),
+        serviceLevel.getValue(),
+        usage.getValue(),
+        billingProvider.getValue(),
+        billingAccountId,
+        beginning,
+        ending,
+        measurementKey.getMetricId());
+  }
 
   @SuppressWarnings("java:S107")
   @Query(