Details
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsInterval.java b/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsInterval.java
index e9c6342..6403b91 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsInterval.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsInterval.java
@@ -19,79 +19,93 @@
package org.killbill.billing.invoice.tree;
import java.util.Collection;
-import java.util.Collections;
-import java.util.Comparator;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
import java.util.UUID;
+import javax.annotation.Nullable;
+
import org.joda.time.LocalDate;
import org.killbill.billing.invoice.api.InvoiceItem;
import org.killbill.billing.invoice.tree.Item.ItemAction;
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.LinkedListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
/**
- * Keeps track of all the items existing on a specified interval.
+ * Keeps track of all the items existing on a specified ItemsNodeInterval
*/
public class ItemsInterval {
- private final UUID targetInvoiceId;
+ // Parent (enclosing) interval
private final ItemsNodeInterval interval;
- private LinkedList<Item> items;
+ private final LinkedList<Item> items;
- public ItemsInterval(final ItemsNodeInterval interval, final UUID targetInvoiceId) {
- this(interval, targetInvoiceId, null);
+ public ItemsInterval(final ItemsNodeInterval interval) {
+ this(interval, null);
}
- public ItemsInterval(final ItemsNodeInterval interval, final UUID targetInvoiceId, final Item initialItem) {
+ public ItemsInterval(final ItemsNodeInterval interval, final Item initialItem) {
this.interval = interval;
- this.targetInvoiceId = targetInvoiceId;
this.items = Lists.newLinkedList();
if (initialItem != null) {
items.add(initialItem);
}
}
- public Item findItem(final UUID targetId) {
- return Iterables.tryFind(items, new Predicate<Item>() {
- @Override
- public boolean apply(final Item input) {
- return input.getId().equals(targetId);
- }
- }).orNull();
- }
-
public List<Item> getItems() {
return items;
}
- public void buildForMissingInterval(final LocalDate startDate, final LocalDate endDate, final List<Item> output, final boolean addRepair) {
- final Item item = createNewItem(startDate, endDate, addRepair);
- if (item != null) {
- output.add(item);
- }
+ public Iterable<Item> get_ADD_items() {
+ return findItems(ItemAction.ADD);
}
- /**
- * Determines what is left based on the mergeMode and the action for each item.
- *
- * @param output
- * @param mergeMode
- * @return whether or not the parent should ignore the interval covered by the child interval
- */
- public void buildFromItems(final List<Item> output, final boolean mergeMode) {
- final Item item = getResultingItem(mergeMode);
- if (item != null) {
- output.add(item);
- }
+ public Iterable<Item> get_CANCEL_items() {
+ return findItems(ItemAction.CANCEL);
+ }
+
+ public Item getCancellingItemIfExists(final UUID targetId) {
+ return Iterables.tryFind(items,
+ new Predicate<Item>() {
+ @Override
+ public boolean apply(final Item input) {
+ return input.getAction() == ItemAction.CANCEL && input.getLinkedId().equals(targetId);
+ }
+ }).orNull();
+ }
+
+ public Item getCancelledItemIfExists(final UUID linkedId) {
+ return Iterables.tryFind(items,
+ new Predicate<Item>() {
+ @Override
+ public boolean apply(final Item input) {
+ return input.getAction() == ItemAction.ADD && input.getId().equals(linkedId);
+ }
+ }).orNull();
+ }
+
+ public NodeInterval getNodeInterval() {
+ return interval;
+ }
+
+ public Item findItem(final UUID targetId) {
+ final Collection<Item> matchingItems = Collections2.<Item>filter(items,
+ new Predicate<Item>() {
+ @Override
+ public boolean apply(final Item input) {
+ return input.getId().equals(targetId);
+ }
+ });
+ Preconditions.checkState(matchingItems.size() < 2, "Too many items matching id='%s' among items='%s'", targetId, items);
+ return matchingItems.size() == 1 ? matchingItems.iterator().next() : null;
}
/**
@@ -119,26 +133,64 @@ public class ItemsInterval {
return items.isEmpty();
}
- public Iterable<Item> get_ADD_items() {
- return Iterables.filter(items, new Predicate<Item>() {
- @Override
- public boolean apply(final Item input) {
- return input.getAction() == ItemAction.ADD;
- }
- });
+ public void add(final Item item) {
+ items.add(item);
}
- public Iterable<Item> get_CANCEL_items() {
- return Iterables.filter(items, new Predicate<Item>() {
- @Override
- public boolean apply(final Item input) {
- return input.getAction() == ItemAction.CANCEL;
- }
- });
+ public void cancelItems(final Item item) {
+ Preconditions.checkState(item.getAction() == ItemAction.ADD);
+ Preconditions.checkState(items.size() == 1);
+ Preconditions.checkState(items.get(0).getAction() == ItemAction.CANCEL);
+ items.clear();
}
- public NodeInterval getNodeInterval() {
- return interval;
+ public void remove(final Item item) {
+ items.remove(item);
+ }
+
+ // Called for missing service periods
+ public void buildForMissingInterval(@Nullable final LocalDate startDate, @Nullable final LocalDate endDate, @Nullable final UUID targetInvoiceId, final Collection<Item> output, final boolean addRepair) {
+ final Item item = createNewItem(startDate, endDate, targetInvoiceId, addRepair);
+ if (item != null) {
+ output.add(item);
+ }
+ }
+
+ // Called on the last node
+ public void buildFromItems(final Collection<Item> output, final boolean mergeMode) {
+ buildForMissingInterval(null, null, null, output, mergeMode);
+ }
+
+ /**
+ * Create a new item based on the existing items and new service period
+ * <p/>
+ * <ul>
+ * <li>During the build phase, 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>During the merge phase, we 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 new item for this service period or null
+ */
+ private Item createNewItem(@Nullable final LocalDate startDate, @Nullable final LocalDate endDate, @Nullable final UUID targetInvoiceId, final boolean mergeMode) {
+ // Find the ADD (build phase) or CANCEL (merge phase) item of this interval
+ final Item item = getResultingItem(mergeMode);
+ if (item == null || startDate == null || endDate == null || targetInvoiceId == null) {
+ return item;
+ }
+
+ // Prorate (build phase) or repair (merge phase) this item, as needed
+ final InvoiceItem proratedInvoiceItem = item.toProratedInvoiceItem(startDate, endDate);
+ if (proratedInvoiceItem == null) {
+ return null;
+ } else {
+ // Keep track of the repaired amount for this item
+ item.incrementCurrentRepairedAmount(proratedInvoiceItem.getAmount().abs());
+ return new Item(proratedInvoiceItem, targetInvoiceId, item.getAction());
+ }
}
private Item getResultingItem(final boolean mergeMode) {
@@ -146,24 +198,18 @@ public class ItemsInterval {
}
private Item getResulting_CANCEL_Item() {
- Preconditions.checkState(items.size() == 0 || items.size() == 1);
+ Preconditions.checkState(items.size() <= 1, "Too many items=%s", items);
return getResulting_CANCEL_ItemNoChecks();
}
private Item getResulting_CANCEL_ItemNoChecks() {
- return Iterables.tryFind(items, new Predicate<Item>() {
- @Override
- public boolean apply(final Item input) {
- return input.getAction() == ItemAction.CANCEL;
- }
- }).orNull();
+ return findItem(ItemAction.CANCEL);
}
private Item getResulting_ADD_Item() {
-
//
// At this point we pruned the items so that we can have either:
- // - 2 items (ADD + CANCEL, where CANCEL does NOT point to ADD item-- otherwise this is a cancelling pair that
+ // - 2 items (ADD + CANCEL, where CANCEL does NOT point to ADD item -- otherwise this is a cancelling pair that
// would have been removed in mergeCancellingPairs logic)
// - 1 ADD item, simple enough we return it
// - 1 CANCEL, there is nothing to return but the period will be ignored by the parent
@@ -172,12 +218,18 @@ public class ItemsInterval {
//
Preconditions.checkState(items.size() <= 2, "Double billing detected: %s", items);
- final Item item = items.size() > 0 && items.get(0).getAction() == ItemAction.ADD ? items.get(0) : null;
+ final Collection<Item> addItems = findItems(ItemAction.ADD);
+ Preconditions.checkState(addItems.size() <= 1, "Double billing detected: %s", items);
+ final Item item = findItem(ItemAction.ADD);
+
+ // Double billing sanity check across nodes
if (item != null) {
final Set<UUID> addItemsCancelled = new HashSet<UUID>();
- if (items.size() > 1) {
- addItemsCancelled.add(items.get(1).getLinkedId());
+ final Item cancelItem = findItem(ItemAction.CANCEL);
+ if (cancelItem != null) {
+ Preconditions.checkState(cancelItem.getLinkedId() != null, "Invalid CANCEL item=%s", cancelItem);
+ addItemsCancelled.add(cancelItem.getLinkedId());
}
final Set<UUID> addItemsToBeCancelled = new HashSet<UUID>();
checkDoubleBilling(addItemsCancelled, addItemsToBeCancelled);
@@ -196,100 +248,39 @@ public class ItemsInterval {
final Item parentAddItem = parentItemsInterval.getResulting_ADD_Item();
if (parentAddItem != null) {
+ Preconditions.checkState(parentAddItem.getId() != null, "Invalid ADD item=%s", parentAddItem);
addItemsToBeCancelled.add(parentAddItem.getId());
}
final Item parentCancelItem = parentItemsInterval.getResulting_CANCEL_ItemNoChecks();
if (parentCancelItem != null) {
+ Preconditions.checkState(parentCancelItem.getLinkedId() != null, "Invalid CANCEL item=%s", parentCancelItem);
addItemsCancelled.add(parentCancelItem.getLinkedId());
}
parentItemsInterval.checkDoubleBilling(addItemsCancelled, addItemsToBeCancelled);
}
- // Just ensure that ADD items precedes CANCEL items
- public void insertSortedItem(final Item item) {
- items.add(item);
- Collections.sort(items, new Comparator<Item>() {
- @Override
- public int compare(final Item o1, final Item o2) {
- if (o1.getAction() == ItemAction.ADD && o2.getAction() == ItemAction.CANCEL) {
- return -1;
- } else if (o1.getAction() == ItemAction.CANCEL && o2.getAction() == ItemAction.ADD) {
- return 1;
- } else {
- return 0;
- }
- }
- });
- }
-
- public void cancelItems(final Item item) {
- Preconditions.checkState(item.getAction() == ItemAction.ADD);
- Preconditions.checkState(items.size() == 1);
- Preconditions.checkState(items.get(0).getAction() == ItemAction.CANCEL);
- items.clear();
+ private Item findItem(final ItemAction itemAction) {
+ final Collection<Item> matchingItems = findItems(itemAction);
+ return matchingItems.size() == 1 ? matchingItems.iterator().next() : null;
}
- public void remove(final Item item) {
- items.remove(item);
+ private Collection<Item> findItems(final ItemAction itemAction) {
+ return Collections2.<Item>filter(items,
+ new Predicate<Item>() {
+ @Override
+ public boolean apply(final Item input) {
+ return input.getAction() == itemAction;
+ }
+ });
}
- public Item getCancellingItemIfExists(final UUID targetId) {
- return Iterables.tryFind(items,
- new Predicate<Item>() {
- @Override
- public boolean apply(final Item input) {
- return input.getAction() == ItemAction.CANCEL && input.getLinkedId().equals(targetId);
- }
- }).orNull();
+ @Override
+ public String toString() {
+ final StringBuilder sb = new StringBuilder("ItemsInterval{");
+ sb.append("items=").append(items);
+ sb.append('}');
+ return sb.toString();
}
-
- public Item getCancelledItemIfExists(final UUID linkedId) {
- return Iterables.tryFind(items,
- new Predicate<Item>() {
- @Override
- public boolean apply(final Item input) {
- return input.getAction() == ItemAction.ADD && input.getId().equals(linkedId);
- }
- }).orNull();
- }
-
- public int size() {
- return items.size();
- }
-
- /**
- * 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(final LocalDate startDate, final LocalDate endDate, final boolean mergeMode) {
-
- final Item item = getResultingItem(mergeMode);
- if (item == null) {
- return null;
- }
-
- final InvoiceItem proratedInvoiceItem = item.toProratedInvoiceItem(startDate, endDate);
- if (proratedInvoiceItem == null) {
- return null;
- }
-
- final Item result = new Item(proratedInvoiceItem, targetInvoiceId, item.getAction());
- if (item.getAction() == ItemAction.CANCEL) {
- item.incrementCurrentRepairedAmount(result.getAmount());
- }
- return result;
- }
-
}
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsNodeInterval.java b/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsNodeInterval.java
index af2ea82..515b88a 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsNodeInterval.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/tree/ItemsNodeInterval.java
@@ -22,6 +22,7 @@ import java.io.IOException;
import java.io.OutputStream;
import java.math.BigDecimal;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
@@ -36,22 +37,23 @@ import org.killbill.billing.util.jackson.ObjectMapper;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.core.JsonGenerator;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+/**
+ * Node in the SubscriptionItemTree
+ */
public class ItemsNodeInterval extends NodeInterval {
- private final UUID targetInvoiceId;
- private ItemsInterval items;
+ private final ItemsInterval items;
- public ItemsNodeInterval(final UUID targetInvoiceId) {
- this.items = new ItemsInterval(this, targetInvoiceId);
- this.targetInvoiceId = targetInvoiceId;
+ public ItemsNodeInterval() {
+ this.items = new ItemsInterval(this);
}
- public ItemsNodeInterval(final ItemsNodeInterval parent, final UUID targetInvoiceId, final Item item) {
+ public ItemsNodeInterval(final ItemsNodeInterval parent, final Item item) {
super(parent, item.getStartDate(), item.getEndDate());
- this.items = new ItemsInterval(this, targetInvoiceId, item);
- this.targetInvoiceId = targetInvoiceId;
+ this.items = new ItemsInterval(this, item);
}
@JsonIgnore
@@ -64,6 +66,33 @@ public class ItemsNodeInterval extends NodeInterval {
}
/**
+ * Add existing item into the tree
+ *
+ * @param newNode an existing item
+ */
+ public void addExistingItem(final ItemsNodeInterval newNode) {
+ Preconditions.checkState(newNode.getItems().size() == 1, "Invalid node=%s", newNode);
+ final Item item = newNode.getItems().get(0);
+
+ addNode(newNode,
+ new AddNodeCallback() {
+ @Override
+ public boolean onExistingNode(final NodeInterval existingNode) {
+ final ItemsInterval existingOrNewNodeItems = ((ItemsNodeInterval) existingNode).getItemsInterval();
+ existingOrNewNodeItems.add(item);
+ // There is no new node added but instead we just populated the list of items for the already existing node
+ return false;
+ }
+
+ @Override
+ public boolean shouldInsertNode(final NodeInterval insertionNode) {
+ // Always want to insert node in the tree when we find the right place.
+ return true;
+ }
+ });
+ }
+
+ /**
* <p/>
* 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
@@ -83,103 +112,24 @@ public class ItemsNodeInterval extends NodeInterval {
* and the goal is to generate the repair items; @see addProposedItem
*
* @param output result list of built items
+ * @param targetInvoiceId
*/
- public void buildForExistingItems(final List<Item> output) {
-
+ public void buildForExistingItems(final Collection<Item> output, final UUID targetInvoiceId) {
// We start by pruning useless entries to simplify the build phase.
- pruneTree();
-
- build(new BuildNodeCallback() {
- @Override
- public void onMissingInterval(final NodeInterval curNode, final LocalDate startDate, final LocalDate endDate) {
- final ItemsInterval items = ((ItemsNodeInterval) curNode).getItemsInterval();
- items.buildForMissingInterval(startDate, endDate, output, false);
- }
-
- @Override
- public void onLastNode(final NodeInterval curNode) {
- final ItemsInterval items = ((ItemsNodeInterval) curNode).getItemsInterval();
- items.buildFromItems(output, false);
- }
- });
- }
-
-
- /**
- * The merge tree is initially constructed by flattening all the existing items and reversing them (CANCEL node).
- * <p/>
- * 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 output result list of built items
- */
- public void mergeExistingAndProposed(final List<Item> output) {
-
- build(new BuildNodeCallback() {
- @Override
- public void onMissingInterval(final NodeInterval curNode, final LocalDate startDate, final LocalDate endDate) {
- final ItemsInterval items = ((ItemsNodeInterval) curNode).getItemsInterval();
- items.buildForMissingInterval(startDate, endDate, output, true);
- }
-
- @Override
- public void onLastNode(final NodeInterval curNode) {
- final ItemsInterval items = ((ItemsNodeInterval) curNode).getItemsInterval();
- items.buildFromItems(output, true);
- }
- }
- );
- }
-
- /**
- * Add existing item into the tree.
- *
- * @param newNode an existing item
- */
- public boolean addExistingItem(final ItemsNodeInterval newNode) {
-
- return addNode(newNode, new AddNodeCallback() {
- @Override
- public boolean onExistingNode(final NodeInterval existingNode) {
- if (!existingNode.isRoot() && newNode.getStart().compareTo(existingNode.getStart()) == 0 && newNode.getEnd().compareTo(existingNode.getEnd()) == 0) {
- final Item item = newNode.getItems().get(0);
- final ItemsInterval existingOrNewNodeItems = ((ItemsNodeInterval) existingNode).getItemsInterval();
- existingOrNewNodeItems.insertSortedItem(item);
- }
- // There is no new node added but instead we just populated the list of items for the already existing node.
- return false;
- }
+ pruneAndValidateTree();
- @Override
- public boolean shouldInsertNode(final NodeInterval insertionNode) {
- // Always want to insert node in the tree when we find the right place.
- return true;
- }
- });
+ build(output, targetInvoiceId, false);
}
/**
- * Add proposed item into the (flattened and reversed) tree.
+ * Add proposed item into the (flattened and reversed) tree
*
* @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.
+ * @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 addProposedItem(final ItemsNodeInterval newNode) {
+ Preconditions.checkState(newNode.getItems().size() == 1, "Invalid node=%s", newNode);
+ final Item item = newNode.getItems().get(0);
return addNode(newNode, new AddNodeCallback() {
@Override
@@ -188,8 +138,6 @@ public class ItemsNodeInterval extends NodeInterval {
return false;
}
- Preconditions.checkState(newNode.getStart().compareTo(existingNode.getStart()) == 0 && newNode.getEnd().compareTo(existingNode.getEnd()) == 0);
- final Item item = newNode.getItems().get(0);
final ItemsInterval existingOrNewNodeItems = ((ItemsNodeInterval) existingNode).getItemsInterval();
existingOrNewNodeItems.cancelItems(item);
// In the merge logic, whether we really insert the node or find an existing node on which to insert items should be seen
@@ -205,17 +153,16 @@ public class ItemsNodeInterval extends NodeInterval {
return false;
}
- final ItemsInterval insertionNodeItems = ((ItemsNodeInterval) insertionNode).getItemsInterval();
- Preconditions.checkState(insertionNodeItems.getItems().size() == 1, "Expected existing node to have only one item");
- final Item insertionNodeItem = insertionNodeItems.getItems().get(0);
- final Item newNodeItem = newNode.getItems().get(0);
+ final List<Item> insertionNodeItems = ((ItemsNodeInterval) insertionNode).getItems();
+ Preconditions.checkState(insertionNodeItems.size() == 1, "Expected existing node to have only one item");
+ final Item insertionNodeItem = insertionNodeItems.get(0);
// If we receive a new proposed that is the same kind as the reversed existing we want to insert it to generate
// a piece of repair
- if (insertionNodeItem.isSameKind(newNodeItem)) {
+ if (insertionNodeItem.isSameKind(item)) {
return true;
- // If not, then keep the proposed outside of the tree.
} else {
+ // If not, then keep the proposed outside of the tree.
return false;
}
}
@@ -223,14 +170,39 @@ public class ItemsNodeInterval extends NodeInterval {
}
/**
+ * The merge tree is initially constructed by flattening all the existing items and reversing them (CANCEL node).
+ * <p/>
+ * 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 output result list of built items
+ */
+ public void mergeExistingAndProposed(final Collection<Item> output, final UUID targetInvoiceId) {
+ build(output, targetInvoiceId, true);
+ }
+
+ /**
* Add the adjustment amount on the item specified by the targetId.
*
* @return linked item if fully adjusted, null otherwise
*/
- public Item addAdjustment(final InvoiceItem item) {
+ public Item addAdjustment(final InvoiceItem item, final UUID targetInvoiceId) {
final UUID targetId = item.getLinkedItemId();
- // TODO we should really be using findNode(adjustmentDate, callback) instead but wrong dates in test creates panic.
final NodeInterval node = findNode(new SearchCallback() {
@Override
public boolean isMatch(final NodeInterval curNode) {
@@ -240,14 +212,13 @@ public class ItemsNodeInterval extends NodeInterval {
Preconditions.checkNotNull(node, "Unable to find item interval for id='%s', tree=%s", targetId, this);
final ItemsInterval targetItemsInterval = ((ItemsNodeInterval) node).getItemsInterval();
- final List<Item> targetItems = targetItemsInterval.getItems();
final Item targetItem = targetItemsInterval.findItem(targetId);
- Preconditions.checkNotNull(targetItem, "Unable to find item with id='%s', items=%s", targetId, targetItems);
+ Preconditions.checkNotNull(targetItem, "Unable to find item with id='%s', itemsInterval=%s", targetId, targetItemsInterval);
final BigDecimal adjustmentAmount = item.getAmount().negate();
if (targetItem.getAmount().compareTo(adjustmentAmount) == 0) {
// Full item adjustment - treat it like a repair
- addExistingItem(new ItemsNodeInterval(this, targetInvoiceId, new Item(item, targetItem.getStartDate(), targetItem.getEndDate(), targetInvoiceId, ItemAction.CANCEL)));
+ addExistingItem(new ItemsNodeInterval(this, new Item(item, targetItem.getStartDate(), targetItem.getEndDate(), targetInvoiceId, ItemAction.CANCEL)));
return targetItem;
} else {
targetItem.incrementAdjustedAmount(adjustmentAmount);
@@ -255,37 +226,20 @@ public class ItemsNodeInterval extends NodeInterval {
}
}
- public void jsonSerializeTree(final ObjectMapper mapper, final OutputStream output) throws IOException {
-
- final JsonGenerator generator = mapper.getFactory().createJsonGenerator(output);
- generator.configure(JsonGenerator.Feature.AUTO_CLOSE_TARGET, false);
-
- walkTree(new WalkCallback() {
-
- private int curDepth = 0;
-
+ private void build(final Collection<Item> output, final UUID targetInvoiceId, final boolean mergeMode) {
+ build(new BuildNodeCallback() {
@Override
- public void onCurrentNode(final int depth, final NodeInterval curNode, final NodeInterval parent) {
- final ItemsNodeInterval node = (ItemsNodeInterval) curNode;
- if (node.isRoot()) {
- return;
- }
+ public void onMissingInterval(final NodeInterval curNode, final LocalDate startDate, final LocalDate endDate) {
+ final ItemsInterval items = ((ItemsNodeInterval) curNode).getItemsInterval();
+ items.buildForMissingInterval(startDate, endDate, targetInvoiceId, output, mergeMode);
+ }
- try {
- if (curDepth < depth) {
- generator.writeStartArray();
- curDepth = depth;
- } else if (curDepth > depth) {
- generator.writeEndArray();
- curDepth = depth;
- }
- generator.writeObject(node);
- } catch (IOException e) {
- throw new RuntimeException("Failed to deserialize tree", e);
- }
+ @Override
+ public void onLastNode(final NodeInterval curNode) {
+ final ItemsInterval items = ((ItemsNodeInterval) curNode).getItemsInterval();
+ items.buildFromItems(output, mergeMode);
}
});
- generator.close();
}
//
@@ -297,7 +251,7 @@ public class ItemsNodeInterval extends NodeInterval {
// whose children completely map the interval (isPartitionedByChildren) and where each child will have a CANCEL item pointing to the ADD.
// When we detect such nodes, we delete both the ADD in the parent interval and the CANCEL in the children (and cleanup the interval if it does not have items)
//
- private void pruneTree() {
+ private void pruneAndValidateTree() {
final NodeInterval root = this;
walkTree(new WalkCallback() {
@Override
@@ -395,7 +349,7 @@ public class ItemsNodeInterval extends NodeInterval {
if (foundFullRepairByParts) {
for (ItemsInterval curItemsInterval : childrenCancellingToBeRemoved.keySet()) {
curItemsInterval.remove(childrenCancellingToBeRemoved.get(curItemsInterval));
- if (curItemsInterval.size() == 0) {
+ if (curItemsInterval.getItems().isEmpty()) {
curNode.removeChild(curItemsInterval.getNodeInterval());
}
}
@@ -409,4 +363,45 @@ public class ItemsNodeInterval extends NodeInterval {
}
});
}
+
+ @VisibleForTesting
+ public void jsonSerializeTree(final ObjectMapper mapper, final OutputStream output) throws IOException {
+ final JsonGenerator generator = mapper.getFactory().createGenerator(output);
+ generator.configure(JsonGenerator.Feature.AUTO_CLOSE_TARGET, false);
+
+ walkTree(new WalkCallback() {
+
+ private int curDepth = 0;
+
+ @Override
+ public void onCurrentNode(final int depth, final NodeInterval curNode, final NodeInterval parent) {
+ final ItemsNodeInterval node = (ItemsNodeInterval) curNode;
+ if (node.isRoot()) {
+ return;
+ }
+
+ try {
+ if (curDepth < depth) {
+ generator.writeStartArray();
+ curDepth = depth;
+ } else if (curDepth > depth) {
+ generator.writeEndArray();
+ curDepth = depth;
+ }
+ generator.writeObject(node);
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to deserialize tree", e);
+ }
+ }
+ });
+ generator.close();
+ }
+
+ @Override
+ public String toString() {
+ final StringBuilder sb = new StringBuilder("ItemsNodeInterval{");
+ sb.append("items=").append(items);
+ sb.append('}');
+ return sb.toString();
+ }
}
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/tree/SubscriptionItemTree.java b/invoice/src/main/java/org/killbill/billing/invoice/tree/SubscriptionItemTree.java
index 70effcf..9c90462 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/tree/SubscriptionItemTree.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/tree/SubscriptionItemTree.java
@@ -38,21 +38,22 @@ import com.google.common.collect.Iterables;
import com.google.common.collect.Ordering;
/**
- * Tree of invoice items for a given subscription.
+ * Tree of invoice items for a given subscription
*/
public class SubscriptionItemTree {
+ private final List<Item> items = new LinkedList<Item>();
+ private final List<Item> existingFullyAdjustedItems = new LinkedList<Item>();
+ private final List<InvoiceItem> existingFixedItems = new LinkedList<InvoiceItem>();
+ private final Map<LocalDate, InvoiceItem> remainingFixedItems = new HashMap<LocalDate, InvoiceItem>();
+ private final List<InvoiceItem> pendingItemAdj = new LinkedList<InvoiceItem>();
+
private final UUID targetInvoiceId;
private final UUID subscriptionId;
- private ItemsNodeInterval root;
- private boolean isBuilt;
- private boolean isMerged;
- private List<Item> items;
- private List<Item> existingFullyAdjustedItems;
- private List<InvoiceItem> existingFixedItems;
- private Map<LocalDate, InvoiceItem> remainingFixedItems;
- private List<InvoiceItem> pendingItemAdj;
+ private ItemsNodeInterval root =new ItemsNodeInterval();
+ private boolean isBuilt = false;
+ private boolean isMerged = false;
private static final Comparator<InvoiceItem> INVOICE_ITEM_COMPARATOR = new Comparator<InvoiceItem>() {
@Override
@@ -74,32 +75,57 @@ public class SubscriptionItemTree {
}
};
+ // targetInvoiceId is the new invoice id being generated
public SubscriptionItemTree(final UUID subscriptionId, final UUID targetInvoiceId) {
this.subscriptionId = subscriptionId;
this.targetInvoiceId = targetInvoiceId;
- this.root = new ItemsNodeInterval(targetInvoiceId);
- this.items = new LinkedList<Item>();
- this.existingFullyAdjustedItems = new LinkedList<Item>();
- this.existingFixedItems = new LinkedList<InvoiceItem>();
- this.remainingFixedItems = new HashMap<LocalDate, InvoiceItem>();
- this.pendingItemAdj = new LinkedList<InvoiceItem>();
- this.isBuilt = false;
}
/**
- * Build the tree to return the list of existing items.
+ * Add an existing item in the tree. A new node is inserted or an existing one updated, if one for the same period already exists.
+ *
+ * @param invoiceItem new existing invoice item on disk.
+ */
+ public void addItem(final InvoiceItem invoiceItem) {
+ Preconditions.checkState(!isBuilt, "Tree already built, unable to add new invoiceItem=%s", invoiceItem);
+
+ switch (invoiceItem.getInvoiceItemType()) {
+ case RECURRING:
+ root.addExistingItem(new ItemsNodeInterval(root, new Item(invoiceItem, targetInvoiceId, ItemAction.ADD)));
+ break;
+
+ case REPAIR_ADJ:
+ root.addExistingItem(new ItemsNodeInterval(root, new Item(invoiceItem, targetInvoiceId, ItemAction.CANCEL)));
+ break;
+
+ case FIXED:
+ existingFixedItems.add(invoiceItem);
+ break;
+
+ case ITEM_ADJ:
+ pendingItemAdj.add(invoiceItem);
+ break;
+
+ default:
+ break;
+ }
+ }
+
+ /**
+ * Build the tree and process adjustments
*/
public void build() {
Preconditions.checkState(!isBuilt);
- for (InvoiceItem item : pendingItemAdj) {
- final Item fullyAdjustedItem = root.addAdjustment(item);
+ for (final InvoiceItem item : pendingItemAdj) {
+ final Item fullyAdjustedItem = root.addAdjustment(item, targetInvoiceId);
if (fullyAdjustedItem != null) {
existingFullyAdjustedItems.add(fullyAdjustedItem);
}
}
pendingItemAdj.clear();
- root.buildForExistingItems(items);
+
+ root.buildForExistingItems(items, targetInvoiceId);
isBuilt = true;
}
@@ -110,68 +136,32 @@ public class SubscriptionItemTree {
*
* @param reverse whether to reverse the existing items (recurring items now show up as CANCEL instead of ADD)
*/
- public void flatten(boolean reverse) {
+ public void flatten(final boolean reverse) {
if (!isBuilt) {
build();
}
- root = new ItemsNodeInterval(targetInvoiceId);
- for (Item item : items) {
+
+ root = new ItemsNodeInterval();
+ for (final Item item : items) {
Preconditions.checkState(item.getAction() == ItemAction.ADD);
- root.addExistingItem(new ItemsNodeInterval(root, targetInvoiceId, new Item(item, reverse ? ItemAction.CANCEL : ItemAction.ADD)));
+ root.addExistingItem(new ItemsNodeInterval(root, new Item(item, reverse ? ItemAction.CANCEL : ItemAction.ADD)));
}
items.clear();
isBuilt = false;
}
- public void buildForMerge() {
- Preconditions.checkState(!isBuilt);
- root.mergeExistingAndProposed(items);
- isBuilt = true;
- isMerged = 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.addExistingItem(new ItemsNodeInterval(root, targetInvoiceId, new Item(invoiceItem, targetInvoiceId, ItemAction.ADD)));
- break;
-
- case REPAIR_ADJ:
- root.addExistingItem(new ItemsNodeInterval(root, targetInvoiceId, new Item(invoiceItem, targetInvoiceId, ItemAction.CANCEL)));
- break;
-
- case FIXED:
- existingFixedItems.add(invoiceItem);
- break;
-
- case ITEM_ADJ:
- pendingItemAdj.add(invoiceItem);
- break;
-
- default:
- break;
- }
- }
-
- /**
- * Merge a new proposed ietm in the tree.
+ * Merge a new proposed item 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, "Tree already built, unable to add new invoiceItem=%s", invoiceItem);
- Preconditions.checkState(!isBuilt);
switch (invoiceItem.getInvoiceItemType()) {
case RECURRING:
- final boolean result = root.addProposedItem(new ItemsNodeInterval(root, targetInvoiceId, new Item(invoiceItem, targetInvoiceId, ItemAction.ADD)));
- if (!result) {
+ final boolean merged = root.addProposedItem(new ItemsNodeInterval(root, new Item(invoiceItem, targetInvoiceId, ItemAction.ADD)));
+ if (!merged) {
items.add(new Item(invoiceItem, targetInvoiceId, ItemAction.ADD));
}
break;
@@ -191,7 +181,14 @@ public class SubscriptionItemTree {
default:
Preconditions.checkState(false, "Unexpected proposed item " + invoiceItem);
}
+ }
+ // Build tree post merge
+ public void buildForMerge() {
+ Preconditions.checkState(!isBuilt, "Tree already built");
+ root.mergeExistingAndProposed(items, targetInvoiceId);
+ isBuilt = true;
+ isMerged = true;
}
/**
@@ -200,6 +197,7 @@ public class SubscriptionItemTree {
* <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() {
@@ -269,10 +267,6 @@ public class SubscriptionItemTree {
}
}
- public UUID getSubscriptionId() {
- return subscriptionId;
- }
-
@Override
public String toString() {
final StringBuilder sb = new StringBuilder("SubscriptionItemTree{");
diff --git a/invoice/src/test/java/org/killbill/billing/invoice/tree/TestSubscriptionItemTree.java b/invoice/src/test/java/org/killbill/billing/invoice/tree/TestSubscriptionItemTree.java
index 853b179..9f592b3 100644
--- a/invoice/src/test/java/org/killbill/billing/invoice/tree/TestSubscriptionItemTree.java
+++ b/invoice/src/test/java/org/killbill/billing/invoice/tree/TestSubscriptionItemTree.java
@@ -100,6 +100,7 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
// Stage II: Try again.. with existing items
existingItems.addAll(tree.getView());
+ assertEquals(existingItems.size(), 5);
tree = new SubscriptionItemTree(subscriptionId, invoiceId);
for (InvoiceItem e : existingItems) {
tree.addItem(e);
@@ -488,6 +489,28 @@ public class TestSubscriptionItemTree extends InvoiceTestSuiteNoDB {
}
}
+ @Test(groups = "fast", description = "https://github.com/killbill/killbill/issues/664")
+ public void testDoubleBillingOnDifferentInvoices() {
+ final LocalDate startDate1 = new LocalDate(2012, 5, 1);
+ final LocalDate endDate = new LocalDate(2012, 6, 1);
+
+ final BigDecimal rate = BigDecimal.TEN;
+ final BigDecimal amount = rate;
+
+ final InvoiceItem recurring1 = new RecurringInvoiceItem(invoiceId, accountId, bundleId, subscriptionId, planName, phaseName, startDate1, endDate, amount, rate, currency);
+ final InvoiceItem recurring2 = new RecurringInvoiceItem(UUID.randomUUID(), accountId, bundleId, subscriptionId, planName, phaseName, startDate1, endDate, amount, rate, currency);
+
+ final SubscriptionItemTree tree = new SubscriptionItemTree(subscriptionId, invoiceId);
+ tree.addItem(recurring1);
+ tree.addItem(recurring2);
+
+ try {
+ tree.build();
+ fail();
+ } catch (final IllegalStateException e) {
+ }
+ }
+
@Test(groups = "fast")
public void testInvalidRepairCausingOverlappingRecurring() {
final LocalDate startDate = new LocalDate(2014, 1, 1);
