killbill-uncached

invoice: fix regression introduced in 38a76121f8e3a4b259422783f47e50a1f6fd6681 The

7/25/2012 9:00:19 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 2a6d781..1a08fd7 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
@@ -235,11 +235,21 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
      */
     void removeDuplicatedInvoiceItems(final List<InvoiceItem> proposedItems,
                                       final List<InvoiceItem> existingInvoiceItems) {
-        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);
+        // We can't just use sets here as order matters (we want to keep duplicated in 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();
+                    break;
+                }
+            }
+        }
     }
 
     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 95cb8c7..8c7b0a8 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,26 +131,74 @@ public class TestDefaultInvoiceGeneratorUnit extends InvoicingTestBase {
     }
 
     @Test(groups = "fast")
-    public void testRemoveDuplicatedInvoiceItemsShouldNotThrowIllegalStateException() {
+    public void testRemoveDuplicatedInvoiceItemsShouldNotThrowIllegalStateExceptionOne() {
         final LocalDate startDate = clock.getUTCToday();
+        final LocalDate endDate = startDate.plusMonths(1);
         final BigDecimal amount = new BigDecimal("12.00");
 
-        // Create a state with multiple duplicates
+        // More items in existing than proposed
+        final List<InvoiceItem> existing = new LinkedList<InvoiceItem>();
+        final InvoiceItem item1 = new FixedPriceInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, amount, currency);
+        final InvoiceItem item2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, amount, amount, currency);
+        existing.add(item1);
+        existing.add(item2);
+
+        final List<InvoiceItem> proposed = new LinkedList<InvoiceItem>();
+        final InvoiceItem other1 = new FixedPriceInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, amount, currency);
+        proposed.add(other1);
+
+        gen.removeDuplicatedInvoiceItems(proposed, existing);
+        assertEquals(existing.size(), 1);
+        assertEquals(proposed.size(), 0);
+    }
 
+    @Test(groups = "fast")
+    public void testRemoveDuplicatedInvoiceItemsShouldNotThrowIllegalStateExceptionTwo() {
+        final LocalDate startDate = clock.getUTCToday();
+        final LocalDate endDate = startDate.plusMonths(1);
+        final BigDecimal amount = new BigDecimal("12.00");
+
+        // More items in proposed than existing
         final List<InvoiceItem> existing = new LinkedList<InvoiceItem>();
         final InvoiceItem item1 = new FixedPriceInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, amount, currency);
         existing.add(item1);
+
+        final List<InvoiceItem> proposed = new LinkedList<InvoiceItem>();
+        final InvoiceItem other1 = new FixedPriceInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, amount, currency);
+        final InvoiceItem other2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, amount, amount, currency);
+        proposed.add(other1);
+        proposed.add(other2);
+
+        gen.removeDuplicatedInvoiceItems(proposed, existing);
+        assertEquals(existing.size(), 0);
+        assertEquals(proposed.size(), 1);
+    }
+
+    @Test(groups = "fast")
+    public void testRemoveDuplicatedInvoiceItemsShouldNotThrowIllegalStateExceptionThree() {
+        final LocalDate startDate = clock.getUTCToday();
+        final LocalDate endDate = startDate.plusMonths(1);
+        final BigDecimal amount = new BigDecimal("12.00");
+
+        // Bunch of duplicated items
+        final List<InvoiceItem> existing = new LinkedList<InvoiceItem>();
+        final InvoiceItem item1 = new FixedPriceInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, amount, currency);
+        final InvoiceItem item2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, amount, amount, currency);
+        existing.add(item1);
+        existing.add(item2);
         existing.add(item1);
 
         final List<InvoiceItem> proposed = new LinkedList<InvoiceItem>();
         final InvoiceItem other1 = new FixedPriceInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, amount, currency);
+        final InvoiceItem other2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, amount, amount, currency);
         proposed.add(item1);
         proposed.add(other1);
         proposed.add(other1);
+        proposed.add(other2);
 
         gen.removeDuplicatedInvoiceItems(proposed, existing);
         assertEquals(existing.size(), 0);
-        assertEquals(proposed.size(), 0);
+        assertEquals(proposed.size(), 1);
     }
 
     @Test(groups = "fast")