killbill-memoizeit

Merge pull request #862 from killbill/fix-jdbc-leaks Code

2/9/2018 5:18:12 AM

Details

diff --git a/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java b/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
index 411e48f..17f8073 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
@@ -304,10 +304,11 @@ public class InvoiceDispatcher {
                     }
                 }
             } else /* Dry run use cases */ {
-
                 final NotificationQueue notificationQueue = notificationQueueService.getNotificationQueue(DefaultInvoiceService.INVOICE_SERVICE_NAME,
                                                                                                           DefaultNextBillingDateNotifier.NEXT_BILLING_DATE_NOTIFIER_QUEUE);
-                final Iterable<NotificationEventWithMetadata<NextBillingDateNotificationKey>> futureNotifications = notificationQueue.getFutureNotificationForSearchKeys(context.getAccountRecordId(), context.getTenantRecordId());
+                final Iterable<NotificationEventWithMetadata<NextBillingDateNotificationKey>> futureNotificationsIterable = notificationQueue.getFutureNotificationForSearchKeys(context.getAccountRecordId(), context.getTenantRecordId());
+                // Copy the results as retrieving the iterator will issue a query each time. This also makes sure the underlying JDBC connection is closed.
+                final List<NotificationEventWithMetadata<NextBillingDateNotificationKey>> futureNotifications = ImmutableList.<NotificationEventWithMetadata<NextBillingDateNotificationKey>>copyOf(futureNotificationsIterable);
 
                 final Map<UUID, DateTime> nextScheduledSubscriptionsEventMap = getNextTransitionsForSubscriptions(billingEvents);
 
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/notification/DefaultNextBillingDatePoster.java b/invoice/src/main/java/org/killbill/billing/invoice/notification/DefaultNextBillingDatePoster.java
index fb269c5..6cfcb83 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/notification/DefaultNextBillingDatePoster.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/notification/DefaultNextBillingDatePoster.java
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
  *
  * The Billing Project licenses this file to you under the Apache License, version 2.0
  * (the "License"); you may not use this file except in compliance with the
@@ -19,6 +19,7 @@
 package org.killbill.billing.invoice.notification;
 
 import java.io.IOException;
+import java.util.Iterator;
 import java.util.UUID;
 
 import org.joda.time.DateTime;
@@ -84,19 +85,27 @@ public class DefaultNextBillingDatePoster implements NextBillingDatePoster {
             final Iterable<NotificationEventWithMetadata<NextBillingDateNotificationKey>> futureNotifications = nextBillingQueue.getFutureNotificationFromTransactionForSearchKeys(internalCallContext.getAccountRecordId(), internalCallContext.getTenantRecordId(), entitySqlDaoWrapperFactory.getHandle().getConnection());
 
             NotificationEventWithMetadata<NextBillingDateNotificationKey> existingNotificationForEffectiveDate = null;
-            for (final NotificationEventWithMetadata<NextBillingDateNotificationKey> input : futureNotifications) {
-                final boolean isEventDryRunForNotifications = input.getEvent().isDryRunForInvoiceNotification() != null ?
-                                                              input.getEvent().isDryRunForInvoiceNotification() : false;
-
-                final LocalDate notificationEffectiveLocaleDate = internalCallContext.toLocalDate(futureNotificationTime);
-                final LocalDate eventEffectiveLocaleDate = internalCallContext.toLocalDate(input.getEffectiveDate());
-
-                if (notificationEffectiveLocaleDate.compareTo(eventEffectiveLocaleDate) == 0 &&
-                    ((isDryRunForInvoiceNotification && isEventDryRunForNotifications) ||
-                     (!isDryRunForInvoiceNotification && !isEventDryRunForNotifications))) {
-                    existingNotificationForEffectiveDate = input;
+            final Iterator<NotificationEventWithMetadata<NextBillingDateNotificationKey>> iterator = futureNotifications.iterator();
+            try {
+                while (iterator.hasNext()) {
+                    final NotificationEventWithMetadata<NextBillingDateNotificationKey> input = iterator.next();
+                    final boolean isEventDryRunForNotifications = input.getEvent().isDryRunForInvoiceNotification() != null ?
+                                                                  input.getEvent().isDryRunForInvoiceNotification() : false;
+
+                    final LocalDate notificationEffectiveLocaleDate = internalCallContext.toLocalDate(futureNotificationTime);
+                    final LocalDate eventEffectiveLocaleDate = internalCallContext.toLocalDate(input.getEffectiveDate());
+
+                    if (notificationEffectiveLocaleDate.compareTo(eventEffectiveLocaleDate) == 0 &&
+                        ((isDryRunForInvoiceNotification && isEventDryRunForNotifications) ||
+                         (!isDryRunForInvoiceNotification && !isEventDryRunForNotifications))) {
+                        existingNotificationForEffectiveDate = input;
+                    }
                 }
+            } finally {
                 // Go through all results to close the connection
+                while (iterator.hasNext()) {
+                    iterator.next();
+                }
             }
 
             if (existingNotificationForEffectiveDate == null) {
diff --git a/invoice/src/main/java/org/killbill/billing/invoice/notification/ParentInvoiceCommitmentPoster.java b/invoice/src/main/java/org/killbill/billing/invoice/notification/ParentInvoiceCommitmentPoster.java
index 43870c3..9a93a82 100644
--- a/invoice/src/main/java/org/killbill/billing/invoice/notification/ParentInvoiceCommitmentPoster.java
+++ b/invoice/src/main/java/org/killbill/billing/invoice/notification/ParentInvoiceCommitmentPoster.java
@@ -1,6 +1,6 @@
 /*
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
  *
  * The Billing Project licenses this file to you under the Apache License, version 2.0
  * (the "License"); you may not use this file except in compliance with the
@@ -18,6 +18,7 @@
 package org.killbill.billing.invoice.notification;
 
 import java.io.IOException;
+import java.util.Iterator;
 import java.util.UUID;
 
 import org.joda.time.DateTime;
@@ -58,14 +59,22 @@ public class ParentInvoiceCommitmentPoster {
             final Iterable<NotificationEventWithMetadata<ParentInvoiceCommitmentNotificationKey>> futureNotifications = commitInvoiceQueue.getFutureNotificationFromTransactionForSearchKeys(internalCallContext.getAccountRecordId(), internalCallContext.getTenantRecordId(), entitySqlDaoWrapperFactory.getHandle().getConnection());
 
             boolean existingFutureNotificationWithSameDate = false;
-            for (final NotificationEventWithMetadata<ParentInvoiceCommitmentNotificationKey> input : futureNotifications) {
-                final LocalDate notificationEffectiveLocaleDate = internalCallContext.toLocalDate(futureNotificationTime);
-                final LocalDate eventEffectiveLocaleDate = internalCallContext.toLocalDate(input.getEffectiveDate());
-
-                if (notificationEffectiveLocaleDate.compareTo(eventEffectiveLocaleDate) == 0) {
-                    existingFutureNotificationWithSameDate = true;
+            final Iterator<NotificationEventWithMetadata<ParentInvoiceCommitmentNotificationKey>> iterator = futureNotifications.iterator();
+            try {
+                while (iterator.hasNext()) {
+                    final NotificationEventWithMetadata<ParentInvoiceCommitmentNotificationKey> input = iterator.next();
+                    final LocalDate notificationEffectiveLocaleDate = internalCallContext.toLocalDate(futureNotificationTime);
+                    final LocalDate eventEffectiveLocaleDate = internalCallContext.toLocalDate(input.getEffectiveDate());
+
+                    if (notificationEffectiveLocaleDate.compareTo(eventEffectiveLocaleDate) == 0) {
+                        existingFutureNotificationWithSameDate = true;
+                    }
                 }
+            } finally {
                 // Go through all results to close the connection
+                while (iterator.hasNext()) {
+                    iterator.next();
+                }
             }
 
             if (!existingFutureNotificationWithSameDate) {
diff --git a/jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/TestResource.java b/jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/TestResource.java
index 172944b..7dd6217 100644
--- a/jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/TestResource.java
+++ b/jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/TestResource.java
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
  *
  * The Billing Project licenses this file to you under the Apache License, version 2.0
  * (the "License"); you may not use this file except in compliance with the
@@ -18,6 +18,8 @@
 
 package org.killbill.billing.jaxrs.resources;
 
+import java.util.Iterator;
+
 import javax.inject.Inject;
 import javax.servlet.ServletRequest;
 import javax.servlet.http.HttpServletRequest;
@@ -279,12 +281,21 @@ public class TestResource extends JaxRsResourceBase {
 
     private boolean areAllNotificationsProcessed(final Long tenantRecordId) {
         int nbNotifications = 0;
-        for (final NotificationQueue notificationQueue : notificationQueueService.getNotificationQueues()) {
-            for (final NotificationEventWithMetadata<NotificationEvent> notificationEvent : notificationQueue.getFutureOrInProcessingNotificationForSearchKey2(null, tenantRecordId)) {
-                if (!notificationEvent.getEffectiveDate().isAfter(clock.getUTCNow())) {
-                    nbNotifications += 1;
+        final Iterator<NotificationQueue> iterator = notificationQueueService.getNotificationQueues().iterator();
+        try {
+            while (iterator.hasNext()) {
+                final NotificationQueue notificationQueue = iterator.next();
+                for (final NotificationEventWithMetadata<NotificationEvent> notificationEvent : notificationQueue.getFutureOrInProcessingNotificationForSearchKey2(null, tenantRecordId)) {
+                    if (!notificationEvent.getEffectiveDate().isAfter(clock.getUTCNow())) {
+                        nbNotifications += 1;
+                    }
                 }
             }
+        } finally {
+            // Go through all results to close the connection
+            while (iterator.hasNext()) {
+                iterator.next();
+            }
         }
         if (nbNotifications != 0) {
             log.info("TestResource: {} queue(s) with more notification(s) to process", nbNotifications);
@@ -294,6 +305,7 @@ public class TestResource extends JaxRsResourceBase {
 
     private boolean areAllBusEventsProcessed(final Long tenantRecordId) {
         final Iterable<BusEventWithMetadata<BusEvent>> availableBusEventForSearchKey2 = persistentBus.getAvailableOrInProcessingBusEventsForSearchKey2(null, tenantRecordId);
+        // This will go through all results to close the connection
         final int nbBusEvents = Iterables.<BusEventWithMetadata<BusEvent>>size(availableBusEventForSearchKey2);
         if (nbBusEvents != 0) {
             log.info("TestResource: at least {} more bus event(s) to process", nbBusEvents);
diff --git a/overdue/src/main/java/org/killbill/billing/overdue/notification/DefaultOverduePosterBase.java b/overdue/src/main/java/org/killbill/billing/overdue/notification/DefaultOverduePosterBase.java
index d06c167..fc14f47 100644
--- a/overdue/src/main/java/org/killbill/billing/overdue/notification/DefaultOverduePosterBase.java
+++ b/overdue/src/main/java/org/killbill/billing/overdue/notification/DefaultOverduePosterBase.java
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
  *
  * The Billing Project licenses this file to you under the Apache License, version 2.0
  * (the "License"); you may not use this file except in compliance with the
@@ -18,6 +18,7 @@
 
 package org.killbill.billing.overdue.notification;
 
+import java.util.Iterator;
 import java.util.UUID;
 
 import org.joda.time.DateTime;
@@ -93,10 +94,18 @@ public abstract class DefaultOverduePosterBase implements OverduePoster {
                 public Void inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
                     final Iterable<NotificationEventWithMetadata<T>> futureNotifications = getFutureNotificationsForAccountInTransaction(entitySqlDaoWrapperFactory, checkOverdueQueue,
                                                                                                                                          clazz, context);
-                    for (final NotificationEventWithMetadata<T> notification : futureNotifications) {
-                        checkOverdueQueue.removeNotificationFromTransaction(entitySqlDaoWrapperFactory.getHandle().getConnection(), notification.getRecordId());
+                    final Iterator<NotificationEventWithMetadata<T>> iterator = futureNotifications.iterator();
+                    try {
+                        while (iterator.hasNext()) {
+                            final NotificationEventWithMetadata<T> notification = iterator.next();
+                            checkOverdueQueue.removeNotificationFromTransaction(entitySqlDaoWrapperFactory.getHandle().getConnection(), notification.getRecordId());
+                        }
+                    } finally {
+                        // Go through all results to close the connection
+                        while (iterator.hasNext()) {
+                            iterator.next();
+                        }
                     }
-
                     return null;
                 }
             });
diff --git a/overdue/src/main/java/org/killbill/billing/overdue/notification/OverdueCheckPoster.java b/overdue/src/main/java/org/killbill/billing/overdue/notification/OverdueCheckPoster.java
index 25d7eff..3fc227a 100644
--- a/overdue/src/main/java/org/killbill/billing/overdue/notification/OverdueCheckPoster.java
+++ b/overdue/src/main/java/org/killbill/billing/overdue/notification/OverdueCheckPoster.java
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
  *
  * The Billing Project licenses this file to you under the Apache License, version 2.0
  * (the "License"); you may not use this file except in compliance with the
@@ -18,6 +18,8 @@
 
 package org.killbill.billing.overdue.notification;
 
+import java.util.Iterator;
+
 import org.joda.time.DateTime;
 import org.killbill.billing.util.cache.CacheControllerDispatcher;
 import org.killbill.billing.util.callcontext.InternalCallContextFactory;
@@ -48,23 +50,32 @@ public class OverdueCheckPoster extends DefaultOverduePosterBase {
         boolean shouldInsertNewNotification = true;
         int minIndexToDeleteFrom = 0;
         int index = 0;
-        for (final NotificationEventWithMetadata<T> cur : futureNotifications) {
-            // Results are ordered by effective date asc
-            if (index == 0) {
-                if (cur.getEffectiveDate().isBefore(futureNotificationTime)) {
-                    // We don't have to insert a new one. For sanity, delete any other future notification
-                    minIndexToDeleteFrom = 1;
-                    shouldInsertNewNotification = false;
-                } else {
-                    // We win - we are before any other already recorded. Delete all others.
-                    minIndexToDeleteFrom = 0;
+        final Iterator<NotificationEventWithMetadata<T>> iterator = futureNotifications.iterator();
+        try {
+            while (iterator.hasNext()) {
+                final NotificationEventWithMetadata<T> cur = iterator.next();
+                // Results are ordered by effective date asc
+                if (index == 0) {
+                    if (cur.getEffectiveDate().isBefore(futureNotificationTime)) {
+                        // We don't have to insert a new one. For sanity, delete any other future notification
+                        minIndexToDeleteFrom = 1;
+                        shouldInsertNewNotification = false;
+                    } else {
+                        // We win - we are before any other already recorded. Delete all others.
+                        minIndexToDeleteFrom = 0;
+                    }
                 }
-            }
 
-            if (minIndexToDeleteFrom <= index) {
-                overdueQueue.removeNotificationFromTransaction(entitySqlDaoWrapperFactory.getHandle().getConnection(), cur.getRecordId());
+                if (minIndexToDeleteFrom <= index) {
+                    overdueQueue.removeNotificationFromTransaction(entitySqlDaoWrapperFactory.getHandle().getConnection(), cur.getRecordId());
+                }
+                index++;
+            }
+        } finally {
+            // Go through all results to close the connection
+            while (iterator.hasNext()) {
+                iterator.next();
             }
-            index++;
         }
 
         return shouldInsertNewNotification;
diff --git a/overdue/src/test/java/org/killbill/billing/overdue/notification/TestDefaultOverdueCheckPoster.java b/overdue/src/test/java/org/killbill/billing/overdue/notification/TestDefaultOverdueCheckPoster.java
index 3f8cd00..9d1aab3 100644
--- a/overdue/src/test/java/org/killbill/billing/overdue/notification/TestDefaultOverdueCheckPoster.java
+++ b/overdue/src/test/java/org/killbill/billing/overdue/notification/TestDefaultOverdueCheckPoster.java
@@ -92,6 +92,7 @@ public class TestDefaultOverdueCheckPoster extends OverdueTestSuiteWithEmbeddedD
         return entitySqlDaoTransactionalJdbiWrapper.execute(new EntitySqlDaoTransactionWrapper<List<NotificationEventWithMetadata<OverdueCheckNotificationKey>>>() {
             @Override
             public List<NotificationEventWithMetadata<OverdueCheckNotificationKey>> inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
+                // This will go through all results to close the connection
                 return ImmutableList.<NotificationEventWithMetadata<OverdueCheckNotificationKey>>copyOf(((OverdueCheckPoster) checkPoster).getFutureNotificationsForAccountInTransaction(entitySqlDaoWrapperFactory, overdueQueue, OverdueCheckNotificationKey.class, internalCallContext));
             }
         });
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java b/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java
index 8cc235e..07b2d12 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/PaymentProcessor.java
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
  *
  * The Billing Project licenses this file to you under the Apache License, version 2.0
  * (the "License"); you may not use this file except in compliance with the
@@ -24,6 +24,7 @@ import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -410,11 +411,19 @@ public class PaymentProcessor extends ProcessorBase {
             final Iterable<NotificationEventWithMetadata<NotificationEvent>> notificationEventWithMetadatas =
                     retryQueue.getFutureNotificationForSearchKeys(internalCallContext.getAccountRecordId(), internalCallContext.getTenantRecordId());
 
-            for (final NotificationEventWithMetadata<NotificationEvent> notificationEvent : notificationEventWithMetadatas) {
-                if (((PaymentRetryNotificationKey) notificationEvent.getEvent()).getAttemptId().equals(lastPaymentAttemptId)) {
-                    retryQueue.removeNotification(notificationEvent.getRecordId());
+            final Iterator<NotificationEventWithMetadata<NotificationEvent>> iterator = notificationEventWithMetadatas.iterator();
+            try {
+                while (iterator.hasNext()) {
+                    final NotificationEventWithMetadata<NotificationEvent> notificationEvent = iterator.next();
+                    if (((PaymentRetryNotificationKey) notificationEvent.getEvent()).getAttemptId().equals(lastPaymentAttemptId)) {
+                        retryQueue.removeNotification(notificationEvent.getRecordId());
+                    }
                 }
+            } finally {
                 // Go through all results to close the connection
+                while (iterator.hasNext()) {
+                    iterator.next();
+                }
             }
         } catch (final NoSuchNotificationQueue noSuchNotificationQueue) {
             log.error("ERROR Loading Notification Queue - " + noSuchNotificationQueue.getMessage());
diff --git a/payment/src/test/java/org/killbill/billing/payment/TestJanitor.java b/payment/src/test/java/org/killbill/billing/payment/TestJanitor.java
index 4b2e002..8cca2de 100644
--- a/payment/src/test/java/org/killbill/billing/payment/TestJanitor.java
+++ b/payment/src/test/java/org/killbill/billing/payment/TestJanitor.java
@@ -1,6 +1,6 @@
 /*
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2018 Groupon, Inc
+ * Copyright 2014-2018 The Billing Project, LLC
  *
  * The Billing Project licenses this file to you under the Apache License, version 2.0
  * (the "License"); you may not use this file except in compliance with the
@@ -20,6 +20,7 @@ package org.killbill.billing.payment;
 import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
@@ -512,11 +513,19 @@ public class TestJanitor extends PaymentTestSuiteWithEmbeddedDB {
                 @Override
                 public Boolean call() throws Exception {
                     boolean completed = true;
-                    for (final NotificationEventWithMetadata<NotificationEvent> notificationEvent : notificationQueueService.getNotificationQueue(DefaultPaymentService.SERVICE_NAME, Janitor.QUEUE_NAME).getFutureOrInProcessingNotificationForSearchKeys(internalCallContext.getAccountRecordId(), internalCallContext.getTenantRecordId())) {
-                        if (!notificationEvent.getEffectiveDate().isAfter(clock.getUTCNow())) {
-                            completed = false;
+                    final Iterator<NotificationEventWithMetadata<NotificationEvent>> iterator = notificationQueueService.getNotificationQueue(DefaultPaymentService.SERVICE_NAME, Janitor.QUEUE_NAME).getFutureOrInProcessingNotificationForSearchKeys(internalCallContext.getAccountRecordId(), internalCallContext.getTenantRecordId()).iterator();
+                    try {
+                        while (iterator.hasNext()) {
+                            final NotificationEventWithMetadata<NotificationEvent> notificationEvent = iterator.next();
+                            if (!notificationEvent.getEffectiveDate().isAfter(clock.getUTCNow())) {
+                                completed = false;
+                            }
                         }
+                    } finally {
                         // Go through all results to close the connection
+                        while (iterator.hasNext()) {
+                            iterator.next();
+                        }
                     }
                     return completed;
                 }