Merge pull request #44 from killbill/do-not-override-items-tier-details

Do not overwrite itemDetails for invoice tax items which contains some value there
killbill · Aug 16, 2024 · 9cda610 · 9cda610
2 parents f482962 + d4de2ac
commit 9cda610
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 10 deletions.
diff --git a/pom.xml b/pom.xml
@@ -47,14 +47,17 @@
         <dependency>
             <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-annotations</artifactId>
+            <version>2.13.4</version>
         </dependency>
         <dependency>
             <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-core</artifactId>
+            <version>2.13.4</version>
         </dependency>
         <dependency>
             <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-databind</artifactId>
+            <version>2.13.4.1</version> <!-- Fixed https://devhub.checkmarx.com/cve-details/CVE-2022-42004/-->
         </dependency>
         <dependency>
             <groupId>com.fasterxml.jackson.datatype</groupId>

diff --git a/src/main/java/org/killbill/billing/plugin/vertex/VertexTaxCalculator.java b/src/main/java/org/killbill/billing/plugin/vertex/VertexTaxCalculator.java
@@ -69,7 +69,9 @@
 import org.slf4j.LoggerFactory;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.HashMultimap;
@@ -106,7 +108,6 @@ public class VertexTaxCalculator extends PluginTaxCalculator {
     private final VertexApiConfigurationHandler vertexApiConfigurationHandler;
     private final VertexDao dao;
     private final Clock clock;
-    private final ObjectMapper objectMapper;
 
     public VertexTaxCalculator(final VertexApiConfigurationHandler vertexApiConfigurationHandler,
                                final VertexDao dao,
@@ -116,7 +117,6 @@ public VertexTaxCalculator(final VertexApiConfigurationHandler vertexApiConfigur
         this.vertexApiConfigurationHandler = vertexApiConfigurationHandler;
         this.clock = clock;
         this.dao = dao;
-        this.objectMapper = new ObjectMapper();
     }
 
     public List<InvoiceItem> compute(final Account account,
@@ -325,10 +325,7 @@ private InvoiceItem createTaxInvoiceItem(final InvoiceItem taxableItem, final UU
             taxRate = calculatedTax.divide(taxableItem.getAmount(), 5, RoundingMode.FLOOR).doubleValue();
         }
 
-        final String taxItemDetails = createTaxItemDetails(ImmutableMap.of("taxRate", taxRate));
-        if (taxItemDetails == null) {
-            return taxItem;
-        }
+        final String taxItemDetails = addTaxRateToItemDetails(taxItem.getItemDetails(), taxRate);
 
         return new PluginInvoiceItem(new Builder<>()
                                              .withId(taxItem.getId())
@@ -361,12 +358,36 @@ private InvoiceItem createTaxInvoiceItem(final InvoiceItem taxableItem, final UU
                                              .validate().build());
     }
 
-    private String createTaxItemDetails(@Nonnull final Map<String, Object> taxItemDetails) {
+    private String addTaxRateToItemDetails(@Nullable final String itemDetails, @Nonnull final Double taxRate) {
+        ObjectNode existingItemsDetailsJson = null;
+        final ObjectMapper objectMapper =  new ObjectMapper();
+
+        if (itemDetails != null && !itemDetails.isEmpty()) {
+            try {
+                final JsonNode jsonNode = objectMapper.readTree(itemDetails);
+                if (!jsonNode.isObject()) {
+                    return itemDetails;
+                }
+                existingItemsDetailsJson =  (ObjectNode) jsonNode;
+            } catch (JsonProcessingException e) {
+                logger.error("Couldn't deserialize the item details: {}", itemDetails, e);
+                return itemDetails;
+            }
+        }
+
+        final Object itemDetailsWithTaxRate;
+        if (existingItemsDetailsJson != null) {
+            existingItemsDetailsJson.put("taxRate", taxRate);
+            itemDetailsWithTaxRate = existingItemsDetailsJson;
+        } else {
+            itemDetailsWithTaxRate = ImmutableMap.of("taxRate", taxRate);
+        }
+
         try {
-            return objectMapper.writeValueAsString(taxItemDetails);
+            return objectMapper.writeValueAsString(itemDetailsWithTaxRate);
         } catch (JsonProcessingException exception) {
-            logger.error("Couldn't serialize the tax item details: {}", taxItemDetails, exception);
-            return null;
+            logger.error("Couldn't serialize the tax item details {} with tax rate: {}", itemDetailsWithTaxRate, taxRate, exception);
+            return itemDetails;
         }
     }
 

diff --git a/src/test/java/org/killbill/billing/plugin/vertex/VertexTaxCalculatorTest.java b/src/test/java/org/killbill/billing/plugin/vertex/VertexTaxCalculatorTest.java
@@ -50,6 +50,9 @@
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyMap;
 import static org.mockito.ArgumentMatchers.argThat;
@@ -251,6 +254,47 @@ public void testTaxEffectiveRate() throws Exception {
         checkTaxItemFields(result.get(0));
     }
 
+    @Test(groups = "fast")
+    public void testTaxItemDetailsWhenUsageDetailsExists() throws Exception {
+        //given
+        final Double taxRate = 0.09975d;
+        given(taxesType.getEffectiveRate()).willReturn(taxRate);
+
+        final String itemDetailsWithTierInfo =
+                "{\"tierDetails\":[{\"tier\":1,\"tierUnit\":\"hour\",\"tierPrice\":0.500000000," +
+                "\"tierBlockSize\":1,\"quantity\":1,\"amount\":0.500000000}],\"amount\":0.500000000}";
+        given(taxableInvoiceItem.getItemDetails()).willReturn(itemDetailsWithTierInfo);
+
+        final String expectedItemDetailsWithTaxRate =
+                "{\"tierDetails\":[{\"tier\":1,\"tierUnit\":\"hour\",\"tierPrice\":0.500000000," +
+                "\"tierBlockSize\":1,\"quantity\":1,\"amount\":0.500000000}],\"amount\":0.500000000," +
+                "\"taxRate\":" + taxRate + "}";
+
+        //when
+        List<InvoiceItem> result = vertexTaxCalculator.compute(account, invoice, true, Collections.emptyList(), tenantContext);
+        ObjectMapper objectMapper = new ObjectMapper();
+        final JsonNode expectedNode = objectMapper.readTree(expectedItemDetailsWithTaxRate);
+        final JsonNode actualNode = objectMapper.readTree(result.get(0).getItemDetails());
+
+        //then
+        assertEquals(expectedNode, actualNode);
+    }
+
+    @Test(groups = "fast")
+    public void testTaxItemDetailsWhenExistingItemDetailsIsNotJson() throws Exception {
+        //given
+        final Double taxRate = 0.09975d;
+        given(taxesType.getEffectiveRate()).willReturn(taxRate);
+        final String invoiceItemDetails = "someItemDetails";
+        given(taxableInvoiceItem.getItemDetails()).willReturn(invoiceItemDetails);
+
+        //when
+        List<InvoiceItem> result = vertexTaxCalculator.compute(account, invoice, true, Collections.emptyList(), tenantContext);
+
+        //then
+        assertEquals(invoiceItemDetails, result.get(0).getItemDetails());
+    }
+
     @Test(groups = "fast")
     public void testComputeWhenTaxEffectiveRateIsNotPresentedByVertex() throws Exception {
         //given