killbill-aplcache

Rework some of the logic for the invoice item tree Javadoc for

2/24/2014 9:49:52 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 4020308..6b02d6c 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
@@ -86,16 +86,11 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
 
         if (existingInvoices != null) {
             for (final Invoice invoice : existingInvoices) {
-
-                final List<InvoiceItem> allSeenItems = new LinkedList<InvoiceItem>();
-                allSeenItems.addAll(invoice.getInvoiceItems());
-
                 for (final InvoiceItem item : invoice.getInvoiceItems()) {
-
                     if (item.getSubscriptionId() == null || // Always include migration invoices, credits, external charges etc.
                         !events.getSubscriptionIdsWithAutoInvoiceOff()
                                .contains(item.getSubscriptionId())) { //don't add items with auto_invoice_off tag
-                        tree.addItem(item, allSeenItems);
+                        tree.addExistingItem(item);
                     }
                 }
             }
@@ -110,7 +105,7 @@ public class DefaultInvoiceGenerator implements InvoiceGenerator {
         final List<InvoiceItem> proposedItems = generateInvoiceItems(invoiceId, accountId, events, adjustedTargetDate, targetCurrency);
 
         tree.mergeWithProposedItems(proposedItems);
-        final List<InvoiceItem> finalItems = tree.getCurrentExistingItemsView();
+        final List<InvoiceItem> finalItems = tree.getResultingItemList();
         invoice.addInvoiceItems(finalItems);
 
         return finalItems.size() != 0 ? invoice : null;
diff --git a/invoice/src/main/java/com/ning/billing/invoice/tree/AccountItemTree.java b/invoice/src/main/java/com/ning/billing/invoice/tree/AccountItemTree.java
index 01e4cc6..8aaa7e7 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/tree/AccountItemTree.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/tree/AccountItemTree.java
@@ -18,6 +18,7 @@ package com.ning.billing.invoice.tree;
 
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
@@ -25,47 +26,88 @@ import java.util.UUID;
 import com.ning.billing.invoice.api.InvoiceItem;
 import com.ning.billing.invoice.api.InvoiceItemType;
 
+import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
 
+/**
+ * Tree of invoice items for a given account.
+ *
+ * <p>It contains a map of <tt>SubscriptionItemTree</tt> and the logic is executed independently for all items
+ * associated to a given subscription. That also means that invoice item adjustment which cross subscriptions
+ * can't be correctly handled when they compete with other forms of adjustments.
+ *
+ * <p>The class is not thread safe, there is no such use case today, and there is a lifecyle to respect:
+ * <ul>
+ * <li>Add existing invoice items
+ * <li>Build the tree,
+ * <li>Merge the proposed list
+ * <li>Retrieves final list
+ * <ul/>
+ */
 public class AccountItemTree {
 
     private final UUID accountId;
     private final Map<UUID, SubscriptionItemTree> subscriptionItemTree;
+    private final List<InvoiceItem> allExistingItems;
+
+    private boolean isBuilt;
 
     public AccountItemTree(final UUID accountId) {
         this.accountId = accountId;
         this.subscriptionItemTree = new HashMap<UUID, SubscriptionItemTree>();
+        this.isBuilt = false;
+        this.allExistingItems = new LinkedList<InvoiceItem>();
     }
 
-    public void addItem(final InvoiceItem item, final List<InvoiceItem> allItems) {
-        if (item.getInvoiceItemType() != InvoiceItemType.RECURRING &&
-            item.getInvoiceItemType() != InvoiceItemType.REPAIR_ADJ &&
-            item.getInvoiceItemType() != InvoiceItemType.FIXED &&
-            item.getInvoiceItemType() != InvoiceItemType.ITEM_ADJ) {
+    /**
+     * build the subscription trees after they have been populated with existing items on disk
+     */
+    public void build() {
+        Preconditions.checkState(!isBuilt);
+        for (SubscriptionItemTree tree : subscriptionItemTree.values()) {
+            tree.build();
+        }
+        isBuilt = true;
+    }
+    /**
+     * Populate tree from existing items on disk
+     *
+     * @param existingItem an item read on disk
+     */
+    public void addExistingItem(final InvoiceItem existingItem) {
+
+        Preconditions.checkState(!isBuilt);
+        if (existingItem.getInvoiceItemType() != InvoiceItemType.RECURRING &&
+            existingItem.getInvoiceItemType() != InvoiceItemType.REPAIR_ADJ &&
+            existingItem.getInvoiceItemType() != InvoiceItemType.FIXED &&
+            existingItem.getInvoiceItemType() != InvoiceItemType.ITEM_ADJ) {
             return;
         }
-        final UUID subscriptionId  = getSubscriptionId(item, allItems);
+
+        allExistingItems.add(existingItem);
+
+        final UUID subscriptionId  = getSubscriptionId(existingItem, allExistingItems);
+        Preconditions.checkNotNull(subscriptionId);
 
         if (!subscriptionItemTree.containsKey(subscriptionId)) {
             subscriptionItemTree.put(subscriptionId, new SubscriptionItemTree(subscriptionId));
         }
         final SubscriptionItemTree tree = subscriptionItemTree.get(subscriptionId);
-        tree.addItem(item);
+        tree.addItem(existingItem);
     }
 
-    public void build() {
-        for (SubscriptionItemTree tree : subscriptionItemTree.values()) {
-            tree.build();
-        }
-    }
-
-
+    /**
+     * Rebuild the new tree by merging current on-disk existing view with new proposed list.
+     *
+     * @param proposedItems list of proposed item that should be merged with current existing view
+     */
     public void mergeWithProposedItems(final List<InvoiceItem> proposedItems) {
 
         for (SubscriptionItemTree tree : subscriptionItemTree.values()) {
             tree.flatten(true);
         }
+        isBuilt = true;
 
         for (InvoiceItem item : proposedItems) {
             final UUID subscriptionId  = getSubscriptionId(item, null);
@@ -82,8 +124,11 @@ public class AccountItemTree {
         }
     }
 
-    public List<InvoiceItem> getCurrentExistingItemsView() {
-
+    /**
+     *
+     * @return the resulting list of items that should be written to disk
+     */
+    public List<InvoiceItem> getResultingItemList() {
         final List<InvoiceItem> result = new ArrayList<InvoiceItem>();
         for (SubscriptionItemTree tree : subscriptionItemTree.values()) {
             final List<InvoiceItem> simplifiedView = tree.getView();
@@ -94,6 +139,10 @@ public class AccountItemTree {
         return result;
     }
 
+    public UUID getAccountId() {
+        return accountId;
+    }
+
     private UUID getSubscriptionId(final InvoiceItem item, final List<InvoiceItem> allItems) {
         if (item.getInvoiceItemType() == InvoiceItemType.RECURRING ||
             item.getInvoiceItemType() == InvoiceItemType.FIXED) {
@@ -107,6 +156,5 @@ public class AccountItemTree {
             }).get();
             return linkedItem.getSubscriptionId();
         }
-
     }
 }
diff --git a/invoice/src/main/java/com/ning/billing/invoice/tree/Item.java b/invoice/src/main/java/com/ning/billing/invoice/tree/Item.java
index eec3032..e820459 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/tree/Item.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/tree/Item.java
@@ -33,6 +33,12 @@ import com.ning.billing.invoice.model.RepairAdjInvoiceItem;
 import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 
+/**
+ * An generic invoice item that contains all pertinent fields regarding of its InvoiceItemType.
+ * <p>
+ * It contains an action that determines what to do when building the tree (whether in normal or merge mode). It also
+ * keeps track of current adjusted and repair amount so subsequent repair can be limited to what is left.
+ */
 public class Item {
 
     private static final int ROUNDING_MODE = InvoicingConfiguration.getRoundingMode();
@@ -76,7 +82,8 @@ public class Item {
         this.amount = item.amount;
         this.rate = item.rate;
         this.currency = item.currency;
-        this.linkedId = this.id; // STEPH called from flatten where linkedId does not exist
+        // In merge mode, the reverse item needs to correctly point to itself (repair of original item)
+        this.linkedId = action == ItemAction.ADD ? item.linkedId : this.id;
         this.createdDate = item.createdDate;
         this.currentRepairedAmount = item.currentRepairedAmount;
         this.adjustedAmount = item.adjustedAmount;
@@ -114,6 +121,7 @@ public class Item {
         int nbTotalDays = Days.daysBetween(startDate, endDate).getDays();
         final boolean prorated = !(newStartDate.compareTo(startDate) == 0 && newEndDate.compareTo(endDate) == 0);
 
+        // Pro-ration is built by using the startDate, endDate and amount of this item instead of using the rate and a potential full period.
         final BigDecimal positiveAmount = prorated ?
                                           InvoiceDateUtils.calculateProrationBetweenDates(newStartDate, newEndDate, nbTotalDays)
                                                           .multiply(amount).setScale(NUMBER_OF_DECIMALS, ROUNDING_MODE) :
@@ -122,6 +130,7 @@ public class Item {
         if (action == ItemAction.ADD) {
             return new RecurringInvoiceItem(id, createdDate, invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, newStartDate, newEndDate, positiveAmount, rate, currency);
         } else {
+            // We first compute the maximum amount after adjustment and that sets the amount limit of how much can be repaired.
             final BigDecimal maxAvailableAmountAfterAdj = amount.subtract(adjustedAmount);
             final BigDecimal maxAvailableAmountForRepair = maxAvailableAmountAfterAdj.subtract(currentRepairedAmount);
             final BigDecimal positiveAmountForRepair = positiveAmount.compareTo(maxAvailableAmountForRepair) <= 0 ? positiveAmount : maxAvailableAmountForRepair;
@@ -167,6 +176,12 @@ public class Item {
         return currency;
     }
 
+    /**
+     * Compare two items to check whether there are the same kind; that is whether or not they build for the same product/plan.
+     *
+     * @param other item to compare with
+     * @return
+     */
     public boolean isSameKind(final Item other) {
 
         final InvoiceItem otherItem = other.toInvoiceItem();
@@ -176,7 +191,6 @@ public class Item {
                // following conditions: same type, subscription, start date. Depending on the catalog configuration, the end
                // date check could also match (e.g. repair from annual to monthly). For that scenario, we need to default
                // to catalog checks (the rate check is a lame check for versioned catalogs).
-
                Objects.firstNonNull(planName, "").equals(Objects.firstNonNull(otherItem.getPlanName(), "")) &&
                Objects.firstNonNull(phaseName, "").equals(Objects.firstNonNull(otherItem.getPhaseName(), "")) &&
                Objects.firstNonNull(rate, BigDecimal.ZERO).compareTo(Objects.firstNonNull(otherItem.getRate(), BigDecimal.ZERO)) == 0;
diff --git a/invoice/src/main/java/com/ning/billing/invoice/tree/ItemsInterval.java b/invoice/src/main/java/com/ning/billing/invoice/tree/ItemsInterval.java
index 18be2ab..fa7bf04 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/tree/ItemsInterval.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/tree/ItemsInterval.java
@@ -20,7 +20,6 @@ import java.math.BigDecimal;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.ListIterator;
@@ -29,7 +28,6 @@ import java.util.UUID;
 
 import org.joda.time.LocalDate;
 
-import com.ning.billing.invoice.api.InvoiceItem;
 import com.ning.billing.invoice.tree.Item.ItemAction;
 
 import com.google.common.base.Preconditions;
@@ -37,6 +35,9 @@ import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 
+/**
+ * Keeps track of all the items existing on a specified interval.
+ */
 public class ItemsInterval {
 
     private final NodeInterval interval;
@@ -84,7 +85,14 @@ public class ItemsInterval {
         }
     }
 
-    public void buildFromItems(final List<Item> output, final boolean isParentRepair, final boolean mergeMode) {
+    /**
+     * Determines what is left based on the mergeMode and the action for each item.
+     *
+     * @param output
+     * @param mergeMode
+     */
+    public void buildFromItems(final List<Item> output, final boolean mergeMode) {
+
         final Set<UUID> repairedIds = new HashSet<UUID>();
         final ListIterator<Item> it = items.listIterator(items.size());
 
@@ -92,21 +100,24 @@ public class ItemsInterval {
             final Item cur = it.previous();
             switch (cur.getAction()) {
                 case ADD:
-                    if (!mergeMode && !repairedIds.contains(cur.getId())) {
-                        output.add(cur);
+                    // Don't consider ADD items in mergeMode as they are only there to specify the bounderies of the repair elements.
+                    if (!mergeMode) {
+                        // If we found a CANCEL item pointing to that item then don't return it as it was repair (full repair scenario)
+                        if (!repairedIds.contains(cur.getId())) {
+                            output.add(cur);
+                        }
                     }
+                    break;
 
-                    if (mergeMode && !isParentRepair) {
+                case CANCEL:
+                    // In merge logic we want to CANCEL (repair) items)
+                    if (mergeMode) {
                         output.add(cur);
                     }
-                    break;
-                case CANCEL:
+                    // In all cases populate the set with the id of target item being repaired
                     if (cur.getLinkedId() != null) {
                         repairedIds.add(cur.getLinkedId());
                     }
-                    if (mergeMode) {
-                        output.add(cur);
-                    }
                     break;
             }
         }
@@ -136,29 +147,37 @@ public class ItemsInterval {
         items.clear();
     }
 
-    // STEPH so complicated...
-    public boolean isRepairNode() {
-       return items.size() == 1 && items.get(0).getAction() == ItemAction.CANCEL;
-    }
-
+    /**
+     * Creates a new item.
+     * <p/>
+     * <ul>
+     * <li>In normal mode, we only consider ADD items. This happens when for instance an existing item was partially repaired
+     * and there is a need to create a new item which represents the part left -- that was not repaired.
+     * <li>In mergeMode, we allow to create new items that are the missing repaired items (CANCEL).
+     * </ul>
+     *
+     * @param startDate start date of the new item to create
+     * @param endDate   end date of the new item to create
+     * @param mergeMode mode to consider.
+     * @return
+     */
     private Item createNewItem(LocalDate startDate, LocalDate endDate, final boolean mergeMode) {
 
         final List<Item> itemToConsider = new LinkedList<Item>();
-        // STEPH flags...
-        buildFromItems(itemToConsider, true, mergeMode);
-
-        Iterator<Item> it = itemToConsider.iterator();
-        while (it.hasNext()) {
-            final Item cur = it.next();
-            if (cur.getAction() == ItemAction.CANCEL && !mergeMode) {
-                continue;
-            }
-            final Item result = new Item(cur.toProratedInvoiceItem(startDate, endDate), cur.getAction());
-            if (cur.getAction() == ItemAction.CANCEL && result != null) {
-                cur.incrementCurrentRepairedAmount(result.getAmount());
-            }
-            return result;
+        buildFromItems(itemToConsider, mergeMode);
+        if (itemToConsider.size() == 0) {
+            return null;
+        }
+
+        Preconditions.checkState(itemToConsider.size() == 1);
+        final Item item = itemToConsider.size() == 1 ? itemToConsider.get(0) : null;
+        Preconditions.checkState((!mergeMode && item.getAction() == ItemAction.ADD) ||
+                                 (mergeMode && item.getAction() == ItemAction.CANCEL));
+
+        final Item result = new Item(item.toProratedInvoiceItem(startDate, endDate), item.getAction());
+        if (item.getAction() == ItemAction.CANCEL && result != null) {
+            item.incrementCurrentRepairedAmount(result.getAmount());
         }
-        return null;
+        return result;
     }
 }
diff --git a/invoice/src/main/java/com/ning/billing/invoice/tree/NodeInterval.java b/invoice/src/main/java/com/ning/billing/invoice/tree/NodeInterval.java
index 6ecdcf6..de7fd1b 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/tree/NodeInterval.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/tree/NodeInterval.java
@@ -48,11 +48,34 @@ public class NodeInterval {
         this.rightSibling = null;
     }
 
-    public void build(final List<Item> output, boolean isParentRepair, boolean mergeMode) {
+    /**
+     * Build the output list from the elements in the tree.
+     * <p/>
+     * In the simple mode, mergeMode = false, there is no limit in the depth of the tree,
+     * and the build strategy is to first consider the lowest child for a given period
+     * and go up the tree adding missing interval if needed. For e.g, one of the possible scenario:
+     * <pre>
+     * D1                                                  D2
+     * |---------------------------------------------------|   Plan P1
+     *       D1'             D2'
+     *       |---------------|/////////////////////////////|   Plan P2, REPAIR
+     *
+     *  In that case we will generate:
+     *  [D1,D1') on Plan P1; [D1', D2') on Plan P2, and [D2', D2) repair item
+     *
+     * <pre/>
+     *
+     * In the merge mode, the strategy is different, the tree is fairly shallow
+     * and the goal is to generate the repair items; @see mergeProposedItem
+     *
+     * @param output    result list of items
+     * @param mergeMode mode used to produce output list
+     */
+    public void build(final List<Item> output, boolean mergeMode) {
 
         // There is no sub-interval, just add our own items.
         if (leftChild == null) {
-            items.buildFromItems(output, isParentRepair, mergeMode);
+            items.buildFromItems(output, mergeMode);
             return;
         }
 
@@ -62,7 +85,7 @@ public class NodeInterval {
             if (curChild.getStart().compareTo(curDate) > 0) {
                 items.buildForMissingInterval(curDate, curChild.getStart(), output, mergeMode);
             }
-            curChild.build(output, isRepairNode(), mergeMode);
+            curChild.build(output, mergeMode);
             curDate = curChild.getEnd();
             curChild = curChild.getRightSibling();
         }
@@ -71,75 +94,29 @@ public class NodeInterval {
         }
     }
 
-    public boolean isItemContained(final Item item) {
-        return (item.getStartDate().compareTo(start) >= 0 &&
-                item.getStartDate().compareTo(end) <= 0 &&
-                item.getEndDate().compareTo(start) >= 0 &&
-                item.getEndDate().compareTo(end) <= 0);
-    }
-
-    public boolean isItemOverlap(final Item item) {
-        return ((item.getStartDate().compareTo(start) < 0 &&
-                 item.getEndDate().compareTo(end) >= 0) ||
-                (item.getStartDate().compareTo(start) <= 0 &&
-                 item.getEndDate().compareTo(end) > 0));
-    }
-
-    public void addAdjustment(final LocalDate adjustementDate, final BigDecimal amount, final UUID linkedId) {
-        NodeInterval node = findNode(adjustementDate, linkedId);
-        if (node == null) {
-            throw new TreeNodeException("Cannot add adjustement for item = " + linkedId + ", date = " + adjustementDate);
-        }
-        node.setAdjustment(amount.negate(), linkedId);
-    }
-
-    private void setAdjustment(final BigDecimal amount, final UUID linkedId) {
-        items.setAdjustment(amount, linkedId);
-    }
-
-    private NodeInterval findNode(final LocalDate date, final UUID targetItemId) {
-        if (!isRoot()) {
-            throw new TreeNodeException("findNode can only be called from root");
-        }
-        return findNodeRecursively2(this, date, targetItemId);
-    }
-
-    private NodeInterval findNodeRecursively(final NodeInterval curNode, final LocalDate date, final UUID targetItemId) {
-        if (date.compareTo(curNode.getStart()) < 0 || date.compareTo(curNode.getEnd()) > 0) {
-            return null;
-        }
-        NodeInterval curChild = curNode.getLeftChild();
-        while (curChild != null) {
-            if (curChild.getStart().compareTo(date) <= 0 && curChild.getEnd().compareTo(date) >= 0) {
-                if (curChild.containsItem(targetItemId)) {
-                    return curChild;
-                } else {
-                    return findNodeRecursively(curChild, date, targetItemId);
-                }
-            }
-            curChild = curChild.getRightSibling();
-        }
-        return null;
-    }
-
-    private NodeInterval findNodeRecursively2(final NodeInterval curNode, final LocalDate date, final UUID targetItemId) {
-
-        if (!curNode.isRoot() && curNode.containsItem(targetItemId)) {
-            return curNode;
-        }
-
-        NodeInterval curChild = curNode.getLeftChild();
-        while (curChild != null) {
-            final NodeInterval result = findNodeRecursively2(curChild, date, targetItemId);
-            if (result != null) {
-                return result;
-            }
-            curChild = curChild.getRightSibling();
-        }
-        return null;
-    }
-
-
+    /**
+     * The merge tree is initially constructed by flattening all the existing items and reversing them (CANCEL node).
+     * That means that if we were to not merge any new proposed items, we would end up with only those reversed existing
+     * items, and they would all end up repaired-- which is what we want.
+     * <p/>
+     * However, if there are new proposed items, then we look to see if they are children one our existing reverse items
+     * so that we can generate the repair pieces missing. For e.g, below is one scenario among so many:
+     * <p/>
+     * <pre>
+     * D1                                                  D2
+     * |---------------------------------------------------| (existing reversed (CANCEL) item
+     *       D1'             D2'
+     *       |---------------| (proposed same plan)
+     * </pre>
+     * In that case we want to generated a repair for [D1, D1') and [D2',D2)
+     * <p/>
+     * Note that this tree is never very deep, only 3 levels max, with exiting at the first level
+     * and proposed that are the for the exact same plan but for different dates below.
+     *
+     * @param newNode a new proposed item
+     * @return true if the item was merged and will trigger a repair or false if the proposed item should be kept as such
+     *         and no repair generated.
+     */
     public boolean mergeProposedItem(final NodeInterval newNode) {
 
         Preconditions.checkState(newNode.getItems().size() == 1, "Expected new node to have only one item");
@@ -152,20 +129,26 @@ public class NodeInterval {
         computeRootInterval(newNode);
 
         if (leftChild == null) {
-            leftChild = newNode;
-            return true;
+            // There is no existing items, only new proposed one, nothing to add in that merge tree
+            if (isRoot()) {
+                return false;
+            } else {
+                // Proposed item is the first child of an existing item with the same product info.
+                leftChild = newNode;
+                return true;
+
+            }
         }
 
         NodeInterval prevChild = null;
         NodeInterval curChild = leftChild;
         do {
-            Preconditions.checkState(!curChild.isItemOverlap(newNodeItem), "Item " + newNodeItem + " overlaps " + curChild);
-
             if (curChild.isItemContained(newNodeItem)) {
                 final Item existingNodeItem = curChild.getItems().get(0);
 
                 Preconditions.checkState(curChild.getItems().size() == 1, "Expected existing node to have only one item");
                 if (existingNodeItem.isSameKind(newNodeItem)) {
+                    // Proposed item has same product info than parent and is contained so insert it at the right place in the tree
                     curChild.mergeProposedItem(newNode);
                     return true;
                 } else {
@@ -173,6 +156,7 @@ public class NodeInterval {
                 }
             }
 
+            // STEPH test for that code path
             if (newNodeItem.getStartDate().compareTo(curChild.getStart()) < 0) {
                 newNode.rightSibling = curChild;
                 if (prevChild == null) {
@@ -182,15 +166,26 @@ public class NodeInterval {
                 }
                 return true;
             }
+
             prevChild = curChild;
             curChild = curChild.rightSibling;
         } while (curChild != null);
 
-        prevChild.rightSibling = newNode;
-        return true;
+        if (isRoot()) {
+            // The new proposed item spans over a new interval, nothing to add in the merge tree
+            return false;
+        } else {
+            prevChild.rightSibling = newNode;
+            return true;
+        }
     }
 
-    public void addNodeInterval(final NodeInterval newNode) {
+    /**
+     * Add an existing item in the tree of items.
+     *
+     * @param newNode new existing item to be added
+     */
+    public void addExistingItem(final NodeInterval newNode) {
         final Item item = newNode.getItems().get(0);
         if (!isRoot() && item.getStartDate().compareTo(start) == 0 && item.getEndDate().compareTo(end) == 0) {
             items.insertSortedItem(item);
@@ -200,8 +195,65 @@ public class NodeInterval {
         addNode(newNode);
     }
 
-    public boolean isRepairNode() {
-        return items.isRepairNode();
+    /**
+     * Add the adjustment amount on the item specified by the targetId.
+     *
+     * @param adjustementDate date of the adjustment
+     * @param amount amount of the adjustment
+     * @param targetId item that has been adjusted
+     */
+    public void addAdjustment(final LocalDate adjustementDate, final BigDecimal amount, final UUID targetId) {
+        NodeInterval node = findNode(adjustementDate, targetId);
+        Preconditions.checkNotNull(node, "Cannot add adjustement for item = " + targetId + ", date = " + adjustementDate);
+        node.setAdjustment(amount.negate(), targetId);
+    }
+
+    public boolean isItemContained(final Item item) {
+        return (item.getStartDate().compareTo(start) >= 0 &&
+                item.getStartDate().compareTo(end) <= 0 &&
+                item.getEndDate().compareTo(start) >= 0 &&
+                item.getEndDate().compareTo(end) <= 0);
+    }
+
+    public boolean isItemOverlap(final Item item) {
+        return ((item.getStartDate().compareTo(start) < 0 &&
+                 item.getEndDate().compareTo(end) >= 0) ||
+                (item.getStartDate().compareTo(start) <= 0 &&
+                 item.getEndDate().compareTo(end) > 0));
+    }
+
+
+
+    public boolean isRoot() {
+        return parent == null;
+    }
+
+    public LocalDate getStart() {
+        return start;
+    }
+
+    public LocalDate getEnd() {
+        return end;
+    }
+
+    public NodeInterval getParent() {
+        return parent;
+    }
+
+    public NodeInterval getLeftChild() {
+        return leftChild;
+    }
+
+    public NodeInterval getRightSibling() {
+        return rightSibling;
+    }
+
+    public List<Item> getItems() {
+        return items.getItems();
+    }
+
+    public boolean containsItem(final UUID targetId) {
+        return items.containsItem(targetId);
     }
 
     // STEPH TODO are parents correctly maintained and/or do we need them?
@@ -216,7 +268,7 @@ public class NodeInterval {
         NodeInterval curChild = leftChild;
         do {
             if (curChild.isItemContained(item)) {
-                curChild.addNodeInterval(newNode);
+                curChild.addExistingItem(newNode);
                 return;
             }
 
@@ -241,6 +293,12 @@ public class NodeInterval {
         prevChild.rightSibling = newNode;
     }
 
+    /**
+     * Since items may be added out of order, there is no guarantee that we don't suddenly had a new node
+     * whose interval emcompasses cuurent node(s). In which case we need to rebalance the tree.
+     *
+     * @param newNode node that triggered a rebalance operation
+     */
     private void rebalance(final NodeInterval newNode) {
 
         final Item item = newNode.getItems().get(0);
@@ -286,35 +344,49 @@ public class NodeInterval {
         this.end = (end == null || end.compareTo(newNode.getEnd()) < 0) ? newNode.getEnd() : end;
     }
 
-    public boolean isRoot() {
-        return parent == null;
-    }
-
-    public LocalDate getStart() {
-        return start;
-    }
-
-    public LocalDate getEnd() {
-        return end;
+    private void setAdjustment(final BigDecimal amount, final UUID linkedId) {
+        items.setAdjustment(amount, linkedId);
     }
 
-    public NodeInterval getParent() {
-        return parent;
+    private NodeInterval findNode(final LocalDate date, final UUID targetItemId) {
+        Preconditions.checkState(isRoot(), "findNode can only be called from root");
+        return findNodeRecursively2(this, date, targetItemId);
     }
 
-    public NodeInterval getLeftChild() {
-        return leftChild;
+    // TODO That method should be use instaed of findNodeRecursively2 to search the node more effectively using the time
+    // but unfortunately that fails because of our test that use the wrong date when doing adjustments.
+    private NodeInterval findNodeRecursively(final NodeInterval curNode, final LocalDate date, final UUID targetItemId) {
+        if (date.compareTo(curNode.getStart()) < 0 || date.compareTo(curNode.getEnd()) > 0) {
+            return null;
+        }
+        NodeInterval curChild = curNode.getLeftChild();
+        while (curChild != null) {
+            if (curChild.getStart().compareTo(date) <= 0 && curChild.getEnd().compareTo(date) >= 0) {
+                if (curChild.containsItem(targetItemId)) {
+                    return curChild;
+                } else {
+                    return findNodeRecursively(curChild, date, targetItemId);
+                }
+            }
+            curChild = curChild.getRightSibling();
+        }
+        return null;
     }
 
-    public NodeInterval getRightSibling() {
-        return rightSibling;
-    }
+    private NodeInterval findNodeRecursively2(final NodeInterval curNode, final LocalDate date, final UUID targetItemId) {
 
-    public List<Item> getItems() {
-        return items.getItems();
-    }
+        if (!curNode.isRoot() && curNode.containsItem(targetItemId)) {
+            return curNode;
+        }
 
-    public boolean containsItem(final UUID targetId) {
-        return items.containsItem(targetId);
+        NodeInterval curChild = curNode.getLeftChild();
+        while (curChild != null) {
+            final NodeInterval result = findNodeRecursively2(curChild, date, targetItemId);
+            if (result != null) {
+                return result;
+            }
+            curChild = curChild.getRightSibling();
+        }
+        return null;
     }
 }
diff --git a/invoice/src/main/java/com/ning/billing/invoice/tree/SubscriptionItemTree.java b/invoice/src/main/java/com/ning/billing/invoice/tree/SubscriptionItemTree.java
index e59c373..49eb390 100644
--- a/invoice/src/main/java/com/ning/billing/invoice/tree/SubscriptionItemTree.java
+++ b/invoice/src/main/java/com/ning/billing/invoice/tree/SubscriptionItemTree.java
@@ -16,15 +16,16 @@
 
 package com.ning.billing.invoice.tree;
 
-import java.util.Collection;
+import java.util.Comparator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.UUID;
 
 import javax.annotation.Nullable;
 
+import org.joda.time.LocalDate;
+
 import com.ning.billing.invoice.api.InvoiceItem;
-import com.ning.billing.invoice.api.InvoiceItemType;
 import com.ning.billing.invoice.tree.Item.ItemAction;
 
 import com.google.common.base.Function;
@@ -32,7 +33,11 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Ordering;
 
+/**
+ * Tree of invoice items for a given subscription.
+ */
 public class SubscriptionItemTree {
 
     private boolean isBuilt;
@@ -46,6 +51,23 @@ public class SubscriptionItemTree {
     private List<InvoiceItem> remainingFixedItems;
     private List<InvoiceItem> pendingItemAdj;
 
+    private static final Comparator<InvoiceItem> INVOICE_ITEM_COMPARATOR = new Comparator<InvoiceItem>() {
+        @Override
+        public int compare(final InvoiceItem o1, final InvoiceItem o2) {
+            int startDateComp = o1.getStartDate().compareTo(o2.getStartDate());
+            if (startDateComp != 0) {
+                return startDateComp;
+            }
+            int itemTypeComp = Integer.compare(o1.getInvoiceItemType().ordinal(), o2.getInvoiceItemType().ordinal());
+            if (itemTypeComp != 0) {
+                return itemTypeComp;
+            }
+            Preconditions.checkState(false, "Unexpected list of items for subscription " + o1.getSubscriptionId());
+            // Never reached...
+            return 0;
+        }
+    };
+
     public SubscriptionItemTree(final UUID subscriptionId) {
         this.subscriptionId = subscriptionId;
         this.root = new NodeInterval();
@@ -56,16 +78,26 @@ public class SubscriptionItemTree {
         this.isBuilt = false;
     }
 
+    /**
+     * Build the tree to return the list of existing items.
+     */
     public void build() {
         Preconditions.checkState(!isBuilt);
         for (InvoiceItem item : pendingItemAdj) {
             root.addAdjustment(item.getStartDate(), item.getAmount(), item.getLinkedItemId());
         }
         pendingItemAdj.clear();
-        root.build(items, false, false);
+        root.build(items, false);
         isBuilt = true;
     }
 
+    /**
+     * Flattens the tree so its depth only has one levl below root -- becomes a list.
+     * <p>
+     * If the tree was not built, it is first built. The list of items is cleared and the state is now reset to unbuilt.
+     *
+     * @param reverse whether to reverse the existing items (recurring items now show up as CANCEL instead of ADD)
+     */
     public void flatten(boolean reverse) {
         if (!isBuilt) {
             build();
@@ -73,7 +105,7 @@ public class SubscriptionItemTree {
         root = new NodeInterval();
         for (Item item : items) {
             Preconditions.checkState(item.getAction() == ItemAction.ADD);
-            root.addNodeInterval(new NodeInterval(root, new Item(item, reverse ? ItemAction.CANCEL : ItemAction.ADD)));
+            root.addExistingItem(new NodeInterval(root, new Item(item, reverse ? ItemAction.CANCEL : ItemAction.ADD)));
         }
         items.clear();
         isBuilt = false;
@@ -81,20 +113,25 @@ public class SubscriptionItemTree {
 
     public void buildForMerge() {
         Preconditions.checkState(!isBuilt);
-        root.build(items, false, true);
+        root.build(items, true);
         isBuilt = true;
     }
 
+    /**
+     * Add an existing item in the tree.
+     *
+     * @param invoiceItem new existing invoice item on disk.
+     */
     public void addItem(final InvoiceItem invoiceItem) {
 
         Preconditions.checkState(!isBuilt);
         switch (invoiceItem.getInvoiceItemType()) {
             case RECURRING:
-                root.addNodeInterval(new NodeInterval(root, new Item(invoiceItem, ItemAction.ADD)));
+                root.addExistingItem(new NodeInterval(root, new Item(invoiceItem, ItemAction.ADD)));
                 break;
 
             case REPAIR_ADJ:
-                root.addNodeInterval(new NodeInterval(root, new Item(invoiceItem, ItemAction.CANCEL)));
+                root.addExistingItem(new NodeInterval(root, new Item(invoiceItem, ItemAction.CANCEL)));
                 break;
 
             case FIXED:
@@ -110,7 +147,11 @@ public class SubscriptionItemTree {
         }
     }
 
-
+    /**
+     * Merge a new proposed ietm in the tree.
+     *
+     * @param invoiceItem new proposed item that should be merged in the existing tree
+     */
     public void mergeProposedItem(final InvoiceItem invoiceItem) {
 
         Preconditions.checkState(!isBuilt);
@@ -140,12 +181,19 @@ public class SubscriptionItemTree {
 
     }
 
+    /**
+     * Can be called prior or after merge with proposed items.
+     * <ul>
+     * <li>When called prior, the merge this gives a flat view of the existing items on disk
+     * <li>When called after the merge with proposed items, this gives the list of items that should now be written to disk -- new fixed, recurring and repair.
+     * </ul>
+     * @return a flat view of the items in the tree.
+     */
     public List<InvoiceItem> getView() {
 
-        // STEPH TODO check that nodeInterval don't overlap or throw. => double billing...
-        final List<InvoiceItem> result = new LinkedList<InvoiceItem>();
-        result.addAll(remainingFixedItems);
-        result.addAll(Collections2.filter(Collections2.transform(items, new Function<Item, InvoiceItem>() {
+        final List<InvoiceItem> tmp = new LinkedList<InvoiceItem>();
+        tmp.addAll(remainingFixedItems);
+        tmp.addAll(Collections2.filter(Collections2.transform(items, new Function<Item, InvoiceItem>() {
             @Override
             public InvoiceItem apply(final Item input) {
                 return input.toInvoiceItem();
@@ -156,9 +204,42 @@ public class SubscriptionItemTree {
                 return input != null;
             }
         }));
+
+        final List<InvoiceItem> result = Ordering.<InvoiceItem>from(INVOICE_ITEM_COMPARATOR).sortedCopy(tmp);
+        checkItemsListState(result);
         return result;
     }
 
+    // Verify there is no double billing, and no double repair (credits)
+    private void checkItemsListState(final List<InvoiceItem> orderedList) {
+
+        LocalDate prevRecurringEndDate = null;
+        LocalDate prevRepairEndDate = null;
+        for (InvoiceItem cur : orderedList) {
+            switch (cur.getInvoiceItemType()) {
+                case FIXED:
+                    break;
+
+                case RECURRING:
+                    if (prevRecurringEndDate != null) {
+                        Preconditions.checkState(prevRecurringEndDate.compareTo(cur.getStartDate()) <= 0);
+                    }
+                    prevRecurringEndDate = cur.getEndDate();
+                    break;
+
+                case REPAIR_ADJ:
+                    if (prevRepairEndDate != null) {
+                        Preconditions.checkState(prevRepairEndDate.compareTo(cur.getStartDate()) <= 0);
+                    }
+                    prevRepairEndDate = cur.getEndDate();
+                    break;
+
+                default:
+                    Preconditions.checkState(false, "Unexpected item type " + cur.getInvoiceItemType());
+            }
+        }
+    }
+
     public UUID getSubscriptionId() {
         return subscriptionId;
     }
diff --git a/invoice/src/test/java/com/ning/billing/invoice/tree/TestSubscriptionItemTree.java b/invoice/src/test/java/com/ning/billing/invoice/tree/TestSubscriptionItemTree.java
index 9692739..f157cdf 100644
--- a/invoice/src/test/java/com/ning/billing/invoice/tree/TestSubscriptionItemTree.java
+++ b/invoice/src/test/java/com/ning/billing/invoice/tree/TestSubscriptionItemTree.java
@@ -31,7 +31,6 @@ import com.ning.billing.invoice.model.RecurringInvoiceItem;
 import com.ning.billing.invoice.model.RepairAdjInvoiceItem;
 
 import com.google.common.collect.Lists;
-import sun.reflect.annotation.ExceptionProxy;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertTrue;
@@ -192,8 +191,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         verifyResult(tree.getView(), expectedResult);
     }
 
-
-
     @Test(groups = "fast")
     public void testBlockAcrossPeriod() {
 
@@ -203,7 +200,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         final LocalDate unblockDate = new LocalDate(2014, 2, 7);
         final LocalDate endDate = new LocalDate(2014, 3, 1);
 
-
         final BigDecimal rate1 = new BigDecimal("12.00");
         final BigDecimal amount1 = rate1;
 
@@ -259,7 +255,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         tree.build();
         verifyResult(tree.getView(), expectedResult);
 
-
         tree = new SubscriptionItemTree(subscriptionId);
         tree.addItem(monthly1);
         tree.addItem(repair);
@@ -277,7 +272,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         verifyResult(tree.getView(), expectedResult);
     }
 
-
     @Test(groups = "fast")
     public void testMonthlyToAnnualWithLeadingProRation() {
 
@@ -353,7 +347,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         verifyResult(tree.getView(), expectedResult);
     }
 
-
     @Test(groups = "fast")
     public void testMergeWithNoExisting() {
 
@@ -437,7 +430,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         final BigDecimal monthlyRate1 = new BigDecimal("12.00");
         final BigDecimal monthlyAmount1 = monthlyRate1;
 
-
         final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId);
         final InvoiceItem monthly1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, monthlyAmount1, monthlyRate1, currency);
         tree.addItem(monthly1);
@@ -454,7 +446,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         verifyResult(tree.getView(), expectedResult);
     }
 
-
     @Test(groups = "fast")
     public void testMergeCancellationWithFinalRepair() {
 
@@ -465,7 +456,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         final BigDecimal monthlyRate1 = new BigDecimal("12.00");
         final BigDecimal monthlyAmount1 = monthlyRate1;
 
-
         final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId);
         final InvoiceItem monthly1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, monthlyAmount1, monthlyRate1, currency);
         tree.addItem(monthly1);
@@ -492,7 +482,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         final BigDecimal monthlyRate1 = new BigDecimal("12.00");
         final BigDecimal monthlyAmount1 = monthlyRate1;
 
-
         final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId);
         final InvoiceItem monthly1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, monthlyAmount1, monthlyRate1, currency);
         tree.addItem(monthly1);
@@ -559,7 +548,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         final BigDecimal monthlyRate2 = new BigDecimal("20.00");
         final BigDecimal monthlyAmount2 = monthlyRate1;
 
-
         final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId);
         final InvoiceItem monthly1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, monthlyAmount1, monthlyRate1, currency);
         tree.addItem(monthly1);
@@ -606,8 +594,8 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         tree.flatten(true);
 
         final InvoiceItem proposed1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, change1, amount1, rate1, currency);
-        final InvoiceItem proposed2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId,  "foo",  "foo", change1, change2, proratedAmount3, rate2, currency);
-        final InvoiceItem proposed3 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId,  "bar",  "bar", change2, endDate, proratedAmount3, rate3, currency);
+        final InvoiceItem proposed2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, "foo", "foo", change1, change2, proratedAmount3, rate2, currency);
+        final InvoiceItem proposed3 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, "bar", "bar", change2, endDate, proratedAmount3, rate3, currency);
         tree.mergeProposedItem(proposed1);
         tree.mergeProposedItem(proposed2);
         tree.mergeProposedItem(proposed3);
@@ -631,7 +619,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         final BigDecimal monthlyAmount = monthlyRate;
         final BigDecimal fixedAmount = new BigDecimal("5.00");
 
-
         final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId);
         final InvoiceItem monthly = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, monthlyAmount, monthlyRate, currency);
         final InvoiceItem fixed = new FixedPriceInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, fixedAmount, currency);
@@ -639,7 +626,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         tree.addItem(fixed);
         tree.flatten(true);
 
-
         final InvoiceItem proposed1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, monthlyAmount, monthlyRate, currency);
         tree.mergeProposedItem(proposed1);
         tree.mergeProposedItem(fixed);
@@ -659,13 +645,11 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         final BigDecimal monthlyAmount = monthlyRate;
         final BigDecimal fixedAmount = new BigDecimal("5.00");
 
-
         final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId);
         final InvoiceItem monthly = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, monthlyAmount, monthlyRate, currency);
         tree.addItem(monthly);
         tree.flatten(true);
 
-
         final InvoiceItem proposed1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, monthlyAmount, monthlyRate, currency);
         final InvoiceItem fixed = new FixedPriceInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, fixedAmount, currency);
         tree.mergeProposedItem(proposed1);
@@ -677,7 +661,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         verifyResult(tree.getView(), expectedResult);
     }
 
-
     @Test(groups = "fast")
     public void testRepairWithSmallItemAdjustment() {
 
@@ -690,7 +673,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         final BigDecimal rate1 = new BigDecimal("12.00");
         final BigDecimal amount1 = rate1;
 
-
         final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId);
         final InvoiceItem initial = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, amount1, rate1, currency);
         final InvoiceItem itemAdj = new ItemAdjInvoiceItem(initial, itemAdjDate, new BigDecimal("-2.00"), currency);
@@ -709,7 +691,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         verifyResult(tree.getView(), expectedResult);
     }
 
-
     @Test(groups = "fast")
     public void testRepairWithLargeItemAdjustment() {
 
@@ -722,7 +703,6 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         final BigDecimal rate1 = new BigDecimal("12.00");
         final BigDecimal amount1 = rate1;
 
-
         final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId);
         final InvoiceItem initial = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endDate, amount1, rate1, currency);
         final InvoiceItem itemAdj = new ItemAdjInvoiceItem(initial, itemAdjDate, new BigDecimal("-10.00"), currency);
@@ -741,6 +721,45 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         verifyResult(tree.getView(), expectedResult);
     }
 
+    @Test(groups = "fast")
+    public void testMergeMonthlyToAnnualWithNoProRation() {
+
+        final LocalDate startDate = new LocalDate(2014, 1, 1);
+        final LocalDate endMonthly1 = new LocalDate(2014, 2, 1);
+        final LocalDate endMonthly2 = new LocalDate(2014, 3, 1);
+        final LocalDate switchToAnnualDate = new LocalDate(2014, 2, 23);
+        final LocalDate endDate = new LocalDate(2015, 2, 23);
+
+        final BigDecimal monthlyRate = new BigDecimal("12.00");
+        final BigDecimal monthlyAmount = monthlyRate;
+
+        final BigDecimal yearlyRate = new BigDecimal("100.00");
+        final BigDecimal yearlyAmount = yearlyRate;
+
+        final InvoiceItem monthly1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endMonthly1, monthlyAmount, monthlyRate, currency);
+        final InvoiceItem monthly2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, endMonthly1, endMonthly2, monthlyAmount, monthlyRate, currency);
+
+        // First test with items in order
+        SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId);
+        tree.addItem(monthly1);
+        tree.addItem(monthly2);
+        tree.flatten(true);
+
+        final InvoiceItem proposed = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, switchToAnnualDate, endDate, yearlyAmount, yearlyRate, currency);
+        final InvoiceItem proposedMonthly1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate, endMonthly1, monthlyAmount, monthlyRate, currency);
+        tree.mergeProposedItem(proposedMonthly1);
+        final InvoiceItem proRatedmonthly2 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, endMonthly1, switchToAnnualDate, monthlyAmount, monthlyRate, currency);
+        tree.mergeProposedItem(proRatedmonthly2);
+        tree.mergeProposedItem(proposed);
+        tree.buildForMerge();
+
+        final List<InvoiceItem> expectedResult = Lists.newLinkedList();
+        final InvoiceItem repair = new RepairAdjInvoiceItem(invoiceId, accountId, switchToAnnualDate, endMonthly2, new BigDecimal("-2.57"), currency, monthly2.getId());
+        expectedResult.add(proposed);
+        expectedResult.add(repair);
+
+        verifyResult(tree.getView(), expectedResult);
+    }
 
     private void verifyResult(final List<InvoiceItem> result, final List<InvoiceItem> expectedResult) {
         assertEquals(result.size(), expectedResult.size());
@@ -749,5 +768,4 @@ public class TestSubscriptionItemTree /* extends InvoiceTestSuiteNoDB  */ {
         }
     }
 
-
 }