killbill-aplcache

invoice: fix buggy implementation of removeDuplicatedInvoiceItems In

7/23/2012 5:06:54 PM

Details

diff --git a/invoice/src/main/java/com/ning/billing/invoice/generator/DefaultInvoiceGenerator.java b/invoice/src/main/java/com/ning/billing/invoice/generator/DefaultInvoiceGenerator.java
index 8842d3d..2a6d781 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/generator/DefaultInvoiceGenerator.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/generator/DefaultInvoiceGenerator.java
@@ -20,9 +20,11 @@ import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.UUID;
 
 import javax.annotation.Nullable;
@@ -56,6 +58,8 @@ import com.ning.billing.invoice.model.RepairAdjInvoiceItem;
 import com.ning.billing.junction.api.BillingEventSet;
 import com.ning.billing.util.clock.Clock;
 
+import com.google.common.collect.Sets;
+import com.google.common.collect.Sets.SetView;
 import com.google.inject.Inject;
 
 public class DefaultInvoiceGenerator implements InvoiceGenerator {
@@ -231,19 +235,11 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
      */
     void removeDuplicatedInvoiceItems(final List<InvoiceItem> proposedItems,
                                       final List<InvoiceItem> existingInvoiceItems) {
-        final Iterator<InvoiceItem> proposedItemIterator = proposedItems.iterator();
-        while (proposedItemIterator.hasNext()) {
-            final InvoiceItem proposedItem = proposedItemIterator.next();
-
-            final Iterator<InvoiceItem> existingItemIterator = existingInvoiceItems.iterator();
-            while (existingItemIterator.hasNext()) {
-                final InvoiceItem existingItem = existingItemIterator.next();
-                if (existingItem.equals(proposedItem)) {
-                    existingItemIterator.remove();
-                    proposedItemIterator.remove();
-                }
-            }
-        }
+        final Set<InvoiceItem> proposedSet = new HashSet<InvoiceItem>(proposedItems);
+        final Set<InvoiceItem> existingSet = new HashSet<InvoiceItem>(existingInvoiceItems);
+        final SetView<InvoiceItem> intersection = Sets.intersection(proposedSet, existingSet);
+        proposedItems.removeAll(intersection);
+        existingInvoiceItems.removeAll(intersection);
     }
 
     void removeCancellingInvoiceItems(final List<InvoiceItem> items) {
diff --git a/invoice/src/test/java/com/ning/billing/invoice/generator/TestDefaultInvoiceGeneratorUnit.java b/invoice/src/test/java/com/ning/billing/invoice/generator/TestDefaultInvoiceGeneratorUnit.java
index 2f119e3..95cb8c7 100644
--- a/invoice/src/test/java/com/ning/billing/invoice/generator/TestDefaultInvoiceGeneratorUnit.java
+++ b/invoice/src/test/java/com/ning/billing/invoice/generator/TestDefaultInvoiceGeneratorUnit.java
@@ -131,6 +131,29 @@ public class TestDefaultInvoiceGeneratorUnit extends InvoicingTestBase {
     }
 
     @Test(groups = "fast")
+    public void testRemoveDuplicatedInvoiceItemsShouldNotThrowIllegalStateException() {
+        final LocalDate startDate = clock.getUTCToday();
+        final BigDecimal amount = new BigDecimal("12.00");
+
+        // Create a state with multiple duplicates
+
+        final List<InvoiceItem> existing = new LinkedList<InvoiceItem>();
+        final InvoiceItem item1 = new FixedPriceInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, amount, currency);
+        existing.add(item1);
+        existing.add(item1);
+
+        final List<InvoiceItem> proposed = new LinkedList<InvoiceItem>();
+        final InvoiceItem other1 = new FixedPriceInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, amount, currency);
+        proposed.add(item1);
+        proposed.add(other1);
+        proposed.add(other1);
+
+        gen.removeDuplicatedInvoiceItems(proposed, existing);
+        assertEquals(existing.size(), 0);
+        assertEquals(proposed.size(), 0);
+    }
+
+    @Test(groups = "fast")
     public void testRemoveDuplicatedInvoiceItemsFixedPrice() {
         final LocalDate startDate = clock.getUTCToday();
         final LocalDate endDate = startDate.plusDays(30);