Issue #1595 Fix expired subscription update retrieval time scheduling

Change-Id: I225bf887d03e43295f4b18eea677bb24c032b8a7

Former-commit-id: 632586a6d2 [formerly 54dd90afdd] [formerly 656601f272] [formerly f6a286da2d [formerly 656601f272 [formerly 26dd90355f6ce2ed5c409a142622cf220754c2ab]]]
Former-commit-id: f6a286da2d
Former-commit-id: fb3f73098ac1d1c9b79e611ae10bc7164b395903 [formerly 776581bc55]
Former-commit-id: 158ce1fe5b

edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/res/spring/bandwidth-datadelivery.xml

@@ -57,6 +57,9 @@
         <property name="subscriptionLatencyCalculator">
             <bean class="com.raytheon.uf.edex.datadelivery.bandwidth.util.SubscriptionValueLatencyCalculator" />
         </property>
+        <property name="subscriptionRescheduleStrategy">
+            <bean class="com.raytheon.uf.edex.datadelivery.bandwidth.util.AlwaysRescheduleSubscriptions" />
+        </property>
     </bean>
     
     <bean id="bandwidthDaoUtil" class="com.raytheon.uf.edex.datadelivery.bandwidth.util.BandwidthDaoUtil">

edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManager.java

@@ -100,6 +100,7 @@ import com.raytheon.uf.edex.datadelivery.bandwidth.util.BandwidthUtil;
  * Jan 28, 2013 1530       djohnson     Unschedule all allocations for a subscription that does not fully schedule.
  * Jan 30, 2013 1501       djohnson     Fix broken calculations for determining required latency.
  * Feb 05, 2013 1580       mpduff       EventBus refactor.
+ * Feb 14, 2013 1595       djohnson     Check with BandwidthUtil whether or not to reschedule subscriptions on update.
  * </pre>
  * 
  * @author dhladky
@@ -414,8 +415,8 @@ abstract class BandwidthManager extends
             // Add the current subscription to the ones BandwidthManager already
             // knows about.
             try {
-                subscriptions.add(bandwidthDao.newBandwidthSubscription(subscription,
+                subscriptions.add(bandwidthDao.newBandwidthSubscription(
-                        retrievalTime));
+                        subscription, retrievalTime));
             } catch (SerializationException e) {
                 statusHandler.error(
                         "Trapped Exception trying to schedule Subscription["
@@ -434,8 +435,9 @@ abstract class BandwidthManager extends
 
         // Retrieve all the current subscriptions by provider, dataset name and
         // base time.
-        List<BandwidthSubscription> subscriptions = bandwidthDao.getBandwidthSubscriptions(
+        List<BandwidthSubscription> subscriptions = bandwidthDao
-                dao.getProvider(), dao.getDataSetName(), retrievalTime);
+                .getBandwidthSubscriptions(dao.getProvider(),
+                        dao.getDataSetName(), retrievalTime);
 
         statusHandler.info("schedule() - Scheduling subscription ["
                 + dao.getName()
@@ -480,8 +482,10 @@ abstract class BandwidthManager extends
                     .getStatus().equals(RetrievalStatus.PROCESSING))
                     && !retrieval.isSubsumed()) {
 
-                BandwidthSubscription bandwidthSubscription = retrieval.getBandwidthSubscription();
+                BandwidthSubscription bandwidthSubscription = retrieval
-                Calendar retrievalTime = bandwidthSubscription.getBaseReferenceTime();
+                        .getBandwidthSubscription();
+                Calendar retrievalTime = bandwidthSubscription
+                        .getBaseReferenceTime();
                 Calendar startTime = BandwidthUtil.copy(retrievalTime);
 
                 int delayMinutes = retrieval.getDataSetAvailablityDelay();
@@ -516,7 +520,8 @@ abstract class BandwidthManager extends
                 // if so, mark the status of the BandwidthReservation as
                 // READY.
                 List<BandwidthDataSetUpdate> z = bandwidthDao
-                        .getBandwidthDataSetUpdate(bandwidthSubscription.getProvider(),
+                        .getBandwidthDataSetUpdate(
+                                bandwidthSubscription.getProvider(),
                                 bandwidthSubscription.getDataSetName(),
                                 bandwidthSubscription.getBaseReferenceTime());
                 if (z.size() > 0) {
@@ -621,8 +626,8 @@ abstract class BandwidthManager extends
         Calendar now = BandwidthUtil.now();
         // Store the AdhocSubscription with a base time of now..
         try {
-            subscriptions.add(bandwidthDao
+            subscriptions.add(bandwidthDao.newBandwidthSubscription(
-                    .newBandwidthSubscription(subscription, now));
+                    subscription, now));
         } catch (SerializationException e) {
             statusHandler.error(
                     "Trapped Exception trying to schedule AdhocSubscription["
@@ -740,7 +745,8 @@ abstract class BandwidthManager extends
                 // BandwidthReservations
                 // already in place for this subscription.
 
-                Subscription old = bandwidthSubscriptions.get(0).getSubscription();
+                Subscription old = bandwidthSubscriptions.get(0)
+                        .getSubscription();
 
                 // Check to see if estimated size changed. If there was a change
                 // to
@@ -750,7 +756,8 @@ abstract class BandwidthManager extends
                 // change the
                 // RetrievalPlan as long as the size stays the same
 
-                if (subscriptionRequiresReschedule(subscription, old)) {
+                if (BandwidthUtil.subscriptionRequiresReschedule(subscription,
+                        old)) {
 
                     // OK, have to remove the old Subscriptions and add the new
                     // ones..
@@ -796,7 +803,8 @@ abstract class BandwidthManager extends
                                 .iterator();
                         while (itr.hasNext()) {
                             bandwidthSubscription = itr.next();
-                            if (oldCycles.contains(bandwidthSubscription.getCycle())) {
+                            if (oldCycles.contains(bandwidthSubscription
+                                    .getCycle())) {
                                 remove.add(bandwidthSubscription);
                                 itr.remove();
                             }
@@ -820,29 +828,6 @@ abstract class BandwidthManager extends
         }
     }
 
-    /**
-     * Determines when a subscription requires rescheduling.
-     * 
-     * @param subscription
-     *            the new subscription
-     * @param old
-     *            the old version
-     * @return true if it requires rescheduling
-     */
-    protected boolean subscriptionRequiresReschedule(Subscription subscription,
-            Subscription old) {
-        // TODO: Do they have to match EXACTLY... probably not but how
-        // much is 'close enough' in bandwidth terms.
-        boolean requiresReschedule = (old.getDataSetSize() != subscription
-                .getDataSetSize())
-        // Priority is different
-                || (old.getPriority() != subscription.getPriority())
-                // Latency is different
-                || (!(old.getLatencyInMinutes() == subscription
-                        .getLatencyInMinutes()));
-        return requiresReschedule;
-    }
-
     /**
      * {@inheritDoc}
      * 
@@ -856,8 +841,8 @@ abstract class BandwidthManager extends
     }
 
     /**
-     * Remove BandwidthSubscription's (and dependent Objects) from any RetrievalPlans
+     * Remove BandwidthSubscription's (and dependent Objects) from any
-     * they are in and adjust the RetrievalPlans accordingly.
+     * RetrievalPlans they are in and adjust the RetrievalPlans accordingly.
      * 
      * @param bandwidthSubscriptions
      *            The subscriptionDao's to remove.
@@ -865,7 +850,8 @@ abstract class BandwidthManager extends
      * @return
      */
     private List<BandwidthAllocation> remove(
-            List<BandwidthSubscription> bandwidthSubscriptions, boolean reschedule) {
+            List<BandwidthSubscription> bandwidthSubscriptions,
+            boolean reschedule) {
 
         List<BandwidthAllocation> unscheduled = new ArrayList<BandwidthAllocation>();
 
@@ -890,7 +876,8 @@ abstract class BandwidthManager extends
 
                 // For each date, get a unique set of provider & dataset name
                 Set<String> providerDataSet = new HashSet<String>();
-                for (BandwidthSubscription bandwidthSubscription : map.get(baseTime)) {
+                for (BandwidthSubscription bandwidthSubscription : map
+                        .get(baseTime)) {
                     String key = bandwidthSubscription.getProvider() + "::"
                             + bandwidthSubscription.getDataSetName();
                     providerDataSet.add(key);
@@ -903,8 +890,9 @@ abstract class BandwidthManager extends
                     String[] key = providerDataSetName.split("::");
                     String provider = key[0];
                     String dataSetName = key[1];
-                    List<BandwidthSubscription> m = bandwidthDao.getBandwidthSubscriptions(
+                    List<BandwidthSubscription> m = bandwidthDao
-                            provider, dataSetName, baseTime);
+                            .getBandwidthSubscriptions(provider, dataSetName,
+                                    baseTime);
 
                     unscheduled.addAll(aggregate(m));
                 }
@@ -980,16 +968,19 @@ abstract class BandwidthManager extends
                 next.add(Calendar.DAY_OF_YEAR, 1);
                 // Since subscriptions are based on cycles in a day, add one day
                 // to the
-                // completed BandwidthSubscription to get the next days retrieval.
+                // completed BandwidthSubscription to get the next days
+                // retrieval.
 
-                // Now check if that BandwidthSubscription has already been scheduled.
+                // Now check if that BandwidthSubscription has already been
-                BandwidthSubscription a = bandwidthDao.getBandwidthSubscription(
+                // scheduled.
-                        dao.getRegistryId(), next);
+                BandwidthSubscription a = bandwidthDao
+                        .getBandwidthSubscription(dao.getRegistryId(), next);
                 if (a == null) {
                     // Create the new BandwidthSubscription record with the next
                     // time..
                     try {
-                        a = bandwidthDao.newBandwidthSubscription(subscription, next);
+                        a = bandwidthDao.newBandwidthSubscription(subscription,
+                                next);
                     } catch (SerializationException e) {
 
                         statusHandler.error(
@@ -1187,7 +1178,8 @@ abstract class BandwidthManager extends
                             + "].  Ignoring list and using the first item.");
         }
 
-        BandwidthSubscription bandwidthSubscription = bandwidthSubscriptions.get(0);
+        BandwidthSubscription bandwidthSubscription = bandwidthSubscriptions
+                .get(0);
         long id = bandwidthSubscription.getId();
 
         final List<BandwidthAllocation> bandwidthAllocations = bandwidthDao
@@ -1494,7 +1486,8 @@ abstract class BandwidthManager extends
         IBandwidthDao fromDao = copyFrom.bandwidthDao;
 
         Set<Subscription> actualSubscriptions = new HashSet<Subscription>();
-        for (BandwidthSubscription subscription : fromDao.getBandwidthSubscriptions()) {
+        for (BandwidthSubscription subscription : fromDao
+                .getBandwidthSubscriptions()) {
             try {
                 Subscription actualSubscription = subscription
                         .getSubscription();

edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/util/AlwaysRescheduleSubscriptions.java

@@ -0,0 +1,54 @@
+/**
+ * This software was developed and / or modified by Raytheon Company,
+ * pursuant to Contract DG133W-05-CQ-1067 with the US Government.
+ * 
+ * U.S. EXPORT CONTROLLED TECHNICAL DATA
+ * This software product contains export-restricted data whose
+ * export/transfer/disclosure is restricted by U.S. law. Dissemination
+ * to non-U.S. persons whether in the United States or abroad requires
+ * an export license or other authorization.
+ * 
+ * Contractor Name:        Raytheon Company
+ * Contractor Address:     6825 Pine Street, Suite 340
+ *                         Mail Stop B8
+ *                         Omaha, NE 68106
+ *                         402.291.0100
+ * 
+ * See the AWIPS II Master Rights File ("Master Rights File.pdf") for
+ * further licensing information.
+ **/
+package com.raytheon.uf.edex.datadelivery.bandwidth.util;
+
+import com.raytheon.uf.common.datadelivery.registry.Subscription;
+
+/**
+ * Implementation of {@link ISubscriptionRescheduleStrategy} that always
+ * reschedules subscriptions on an update.
+ * 
+ * <pre>
+ * 
+ * SOFTWARE HISTORY
+ * 
+ * Date         Ticket#    Engineer    Description
+ * ------------ ---------- ----------- --------------------------
+ * Feb 14, 2013 1595       djohnson     Initial creation
+ * 
+ * </pre>
+ * 
+ * @author djohnson
+ * @version 1.0
+ */
+
+public class AlwaysRescheduleSubscriptions implements
+        ISubscriptionRescheduleStrategy {
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public boolean subscriptionRequiresReschedule(Subscription subscription,
+            Subscription old) {
+        return true;
+    }
+
+}

edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/util/BandwidthDaoUtil.java

@@ -37,8 +37,8 @@ import com.raytheon.uf.common.status.UFStatus.Priority;
 import com.raytheon.uf.common.time.util.TimeUtil;
 import com.raytheon.uf.edex.datadelivery.bandwidth.dao.BandwidthAllocation;
 import com.raytheon.uf.edex.datadelivery.bandwidth.dao.BandwidthDataSetUpdate;
-import com.raytheon.uf.edex.datadelivery.bandwidth.dao.IBandwidthDao;
 import com.raytheon.uf.edex.datadelivery.bandwidth.dao.BandwidthSubscription;
+import com.raytheon.uf.edex.datadelivery.bandwidth.dao.IBandwidthDao;
 import com.raytheon.uf.edex.datadelivery.bandwidth.retrieval.RetrievalManager;
 import com.raytheon.uf.edex.datadelivery.bandwidth.retrieval.RetrievalPlan;
 import com.raytheon.uf.edex.datadelivery.bandwidth.retrieval.RetrievalStatus;
@@ -55,6 +55,7 @@ import com.raytheon.uf.edex.datadelivery.bandwidth.retrieval.RetrievalStatus;
  * ------------ ---------- ----------- --------------------------
  * Oct 24, 2012 1286       djohnson     Extract methods from {@link BandwidthUtil}.
  * Dec 11, 2012 1286       djohnson     FULFILLED allocations are not in the retrieval plan either.
+ * Feb 14, 2013 1595       djohnson     Fix not using calendar copies, and backwards max/min operations.
  * 
  * </pre>
  * 
@@ -186,18 +187,16 @@ public class BandwidthDaoUtil {
         } else {
             // If there is a start and end time, then modify the start and
             // end times to 'fit' within the RetrievalPlan times
-            subscriptionStartDate = BandwidthUtil.max(
+            subscriptionStartDate = BandwidthUtil.copy(BandwidthUtil.max(
-                    subscription.getSubscriptionEnd(), planEnd);
+                    subscription.getSubscriptionStart(), planStart));
-            subscriptionEndDate = BandwidthUtil.min(
+            subscriptionEndDate = BandwidthUtil.copy(BandwidthUtil.min(
-                    subscription.getSubscriptionStart(), planStart);
+                    subscription.getSubscriptionEnd(), planEnd));
         }
 
         // Create a Set of Calendars for all the baseReferenceTimes that a
         // Subscription can contain...
-        subscriptionStartDate.set(Calendar.HOUR_OF_DAY, 0);
+        TimeUtil.minCalendarFields(subscriptionStartDate, Calendar.MILLISECOND,
-        subscriptionStartDate.set(Calendar.MINUTE, 0);
+                Calendar.SECOND, Calendar.MINUTE, Calendar.HOUR_OF_DAY);
-        subscriptionStartDate.set(Calendar.SECOND, 0);
-        subscriptionStartDate.set(Calendar.MILLISECOND, 0);
 
         outerloop: while (!subscriptionStartDate.after(subscriptionEndDate)) {
 

edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/util/BandwidthUtil.java

@@ -25,6 +25,7 @@ import com.raytheon.uf.edex.datadelivery.bandwidth.dao.BandwidthSubscription;
  *                                     use availability delay to determine which starting hours to schedule.
  * Nov 09, 2012 1286       djohnson    Separate DAO utility methods from general utility.
  * Dec 11, 2012 1403       djohnson    No longer valid to run without bandwidth management.
+ * Feb 14, 2013 1595       djohnson    Use subscription rescheduling strategy.
  * 
  * </pre>
  * 
@@ -51,6 +52,8 @@ public class BandwidthUtil {
 
     private ISubscriptionLatencyCalculator subscriptionLatencyCalculator;
 
+    private ISubscriptionRescheduleStrategy subscriptionRescheduleStrategy;
+
     private BandwidthUtil() {
     };
 
@@ -151,6 +154,15 @@ public class BandwidthUtil {
         this.subscriptionLatencyCalculator = subscriptionLatencyCalculator;
     }
 
+    /**
+     * @param subscriptionRescheduleStrategy
+     *            the subscriptionRescheduleStrategy to set
+     */
+    public void setSubscriptionRescheduleStrategy(
+            ISubscriptionRescheduleStrategy subscriptionRescheduleStrategy) {
+        this.subscriptionRescheduleStrategy = subscriptionRescheduleStrategy;
+    }
+
     /**
      * @return the instance
      */
@@ -277,4 +289,19 @@ public class BandwidthUtil {
     public static long convertBytesToKilobytes(long bytes) {
         return bytes / BandwidthUtil.BYTES_PER_KILOBYTE;
     }
+
+    /**
+     * Check whether a subscription should be rescheduled on an update.
+     * 
+     * @param subscription
+     *            the subscription
+     * @param old
+     *            the old version
+     * @return true if the subscription should be rescheduled
+     */
+    public static boolean subscriptionRequiresReschedule(
+            Subscription subscription, Subscription old) {
+        return instance.subscriptionRescheduleStrategy
+                .subscriptionRequiresReschedule(subscription, old);
+    }
 }

edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/util/ISubscriptionRescheduleStrategy.java

@@ -0,0 +1,54 @@
+/**
+ * This software was developed and / or modified by Raytheon Company,
+ * pursuant to Contract DG133W-05-CQ-1067 with the US Government.
+ * 
+ * U.S. EXPORT CONTROLLED TECHNICAL DATA
+ * This software product contains export-restricted data whose
+ * export/transfer/disclosure is restricted by U.S. law. Dissemination
+ * to non-U.S. persons whether in the United States or abroad requires
+ * an export license or other authorization.
+ * 
+ * Contractor Name:        Raytheon Company
+ * Contractor Address:     6825 Pine Street, Suite 340
+ *                         Mail Stop B8
+ *                         Omaha, NE 68106
+ *                         402.291.0100
+ * 
+ * See the AWIPS II Master Rights File ("Master Rights File.pdf") for
+ * further licensing information.
+ **/
+package com.raytheon.uf.edex.datadelivery.bandwidth.util;
+
+import com.raytheon.uf.common.datadelivery.registry.Subscription;
+
+/**
+ * Interface defining whether a subscription requires a reschedule.
+ * 
+ * <pre>
+ * 
+ * SOFTWARE HISTORY
+ * 
+ * Date         Ticket#    Engineer    Description
+ * ------------ ---------- ----------- --------------------------
+ * Feb 14, 2013 1595       djohnson     Initial creation
+ * 
+ * </pre>
+ * 
+ * @author djohnson
+ * @version 1.0
+ */
+public interface ISubscriptionRescheduleStrategy {
+
+    /**
+     * Check whether a subscription should be rescheduled on an update.
+     * 
+     * @param subscription
+     *            the subscription
+     * @param old
+     *            the old version
+     * @return true if the subscription should be rescheduled
+     */
+    boolean subscriptionRequiresReschedule(Subscription subscription,
+            Subscription old);
+
+}

tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManagerIntTest.java

@@ -19,8 +19,10 @@
  **/
 package com.raytheon.uf.edex.datadelivery.bandwidth;
 
+import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
@@ -95,6 +97,7 @@ import com.raytheon.uf.edex.datadelivery.retrieval.RetrievalManagerNotifyEvent;
  * Jan 25, 2013 1528       djohnson     Compare priorities as primitive ints.
  * Jan 28, 2013 1530       djohnson     Test that all allocations are unscheduled for subscription that doesn't fully schedule.
  * Jan 30, 2013 1501       djohnson     Fix broken calculations for determining required latency.
+ * Feb 14, 2013 1595       djohnson     Fix expired subscription updates that weren't scheduling retrievals.
  * 
  * </pre>
  * 
@@ -395,6 +398,71 @@ public class BandwidthManagerIntTest extends AbstractBandwidthManagerIntTest {
         }
     }
 
+    /**
+     * Redmine #1595: A subscription was created...and then expired. The
+     * subscription was updated by extending the Expiration Date only. The
+     * subscription, however, was never scheduled (e.g., in the Bandwidth
+     * Utilization graph). Data displayed in D2D failed to update. However, the
+     * subscription (and data) did begin updating after editing the subscription
+     * and adding another parameter.
+     */
+    @Test
+    public void expiredSubscriptionUpdatedToNonExpiredIsScheduled()
+            throws Exception {
+
+        final Date yesterday = new Date(TimeUtil.currentTimeMillis() - TimeUtil.MILLIS_PER_DAY);
+        final Date oneHourAgo = new Date(TimeUtil.currentTimeMillis() - TimeUtil.MILLIS_PER_HOUR);
+
+        Subscription subscription = createSubscriptionThatFillsHalfABucket();
+        subscription.setSubscriptionStart(yesterday);
+        subscription.setSubscriptionEnd(oneHourAgo);
+
+        bandwidthManager.subscriptionUpdated(subscription);
+
+        // Make sure nothing is scheduled when the subscription is expired
+        assertThat(bandwidthDao.getBandwidthAllocations(subscription.getRoute()),
+                is(empty()));
+
+        // No longer expired
+        subscription.setSubscriptionEnd(new Date(TimeUtil.currentTimeMillis()
+                + TimeUtil.MILLIS_PER_WEEK));
+
+        bandwidthManager.subscriptionUpdated(subscription);
+
+        assertThat(
+                bandwidthDao.getBandwidthAllocations(subscription.getRoute()),
+                is(not(empty())));
+    }
+
+    @Test
+    public void subscriptionUpdatedToExpiredHasAllocationsRemoved()
+            throws Exception {
+
+        Subscription subscription = createSubscriptionThatFillsHalfABucket();
+
+        bandwidthManager.subscriptionUpdated(subscription);
+
+        assertThat(
+                bandwidthDao.getBandwidthAllocations(subscription.getRoute()),
+                is(not(empty())));
+
+        final Date yesterday = new Date(TimeUtil.currentTimeMillis()
+                - TimeUtil.MILLIS_PER_DAY);
+        final Date oneHourAgo = new Date(TimeUtil.currentTimeMillis()
+                - TimeUtil.MILLIS_PER_HOUR);
+
+        // Updated to expired
+        subscription.setSubscriptionStart(yesterday);
+        subscription.setSubscriptionEnd(oneHourAgo);
+
+        bandwidthManager.subscriptionUpdated(subscription);
+
+        // Make sure nothing is scheduled when the subscription is expired
+        assertThat(
+                bandwidthDao.getBandwidthAllocations(subscription.getRoute()),
+                is(empty()));
+    }
+
     @Test
     public void testScheduleSubscriptionWithHigherPrioritySetsOtherAllocationsToUnscheduled() {
 

tests/unit/com/raytheon/uf/edex/datadelivery/bandwidth/util/BandwidthDaoUtilTest.java

@@ -19,6 +19,7 @@
  **/
 package com.raytheon.uf.edex.datadelivery.bandwidth.util;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -27,6 +28,9 @@ import static org.mockito.Mockito.when;
 
 import java.util.Arrays;
 import java.util.Calendar;
+import java.util.Collection;
+import java.util.Date;
+import java.util.List;
 import java.util.SortedSet;
 import java.util.TreeSet;
 
@@ -36,7 +40,7 @@ import org.junit.Test;
 
 import com.raytheon.uf.common.datadelivery.registry.Network;
 import com.raytheon.uf.common.datadelivery.registry.Subscription;
-import com.raytheon.uf.common.datadelivery.registry.SubscriptionFixture;
+import com.raytheon.uf.common.datadelivery.registry.SubscriptionBuilder;
 import com.raytheon.uf.common.localization.PathManagerFactoryTest;
 import com.raytheon.uf.common.time.util.TimeUtil;
 import com.raytheon.uf.common.time.util.TimeUtilTest;
@@ -60,6 +64,7 @@ import com.raytheon.uf.edex.datadelivery.bandwidth.retrieval.RetrievalStatus;
  * ------------ ---------- ----------- --------------------------
  * Oct 24, 2012 1286       djohnson     Initial creation
  * Feb 07, 2013 1543       djohnson     Remove unnecessary test setup methods.
+ * Feb 14, 2013 1595       djohnson     Fix retrieval plan/subscription time intersections.
  * 
  * </pre>
  * 
@@ -104,11 +109,10 @@ public class BandwidthDaoUtilTest {
     @Test
     public void testGetRetrievalTimesReturnsBaseReferenceTimesInPlanWindow() {
         // Make sure the subscription is "active" within the plan period
-        Subscription subscription = SubscriptionFixture.INSTANCE.get();
+        Subscription subscription = new SubscriptionBuilder()
-        subscription.setActivePeriodStart(plan.getPlanStart().getTime());
+                .withActivePeriodStart(plan.getPlanStart().getTime())
-        subscription.setActivePeriodEnd(plan.getPlanEnd().getTime());
+                .withActivePeriodEnd(plan.getPlanEnd().getTime())
-        subscription.setSubscriptionStart(TimeUtil.newImmutableDate());
+                .withSubscriptionStart(TimeUtil.newImmutableDate()).build();
-        subscription.setSubscriptionEnd(null);
         subscription.getTime().setCycleTimes(
                 Arrays.asList(Integer.valueOf(9), Integer.valueOf(0)));
 
@@ -117,22 +121,66 @@ public class BandwidthDaoUtilTest {
 
         SortedSet<Calendar> subscriptionTimes = new TreeSet<Calendar>();
 
-        subscriptionTimes = bandwidthDaoUtil
+        subscriptionTimes = bandwidthDaoUtil.getRetrievalTimes(subscription,
-                .getRetrievalTimes(subscription, cycles, plan,
+                cycles, plan, subscriptionTimes);
-                        subscriptionTimes);
 
-        // Using millis to verify the returned retrieval times is a bit
+        final List<Integer> daysOfTheYear = Arrays.asList(3, 4);
-        // unpleasant to look at, but it'll do for now
+        verifySubscriptionTimesContainsCyclesForSpecifiedDays(daysOfTheYear,
-        Calendar cal = TimeUtil.newCalendar();
+                cycles, subscriptionTimes);
-        for (long millis : new long[] { 194400000L, 226800000L, 280800000L,
+    }
-                313200000L }) {
-            cal.setTimeInMillis(millis);
-            assertTrue(
-                    "Expected to find retrieval time of "
-                            + BandwidthUtil.format(cal),
-                    subscriptionTimes.contains(cal));
-        }
 
+    @Test
+    public void testGetRetrievalTimesUsesSubscriptionStartWhenItIsMostRestrictive() {
+        final Date startsTwoDaysIntoPlan = new Date(plan.getPlanStart()
+                .getTimeInMillis() + TimeUtil.MILLIS_PER_DAY * 2);
+
+        Subscription subscription = new SubscriptionBuilder()
+                .withSubscriptionStart(startsTwoDaysIntoPlan)
+                .withSubscriptionEnd(plan.getPlanEnd().getTime()).build();
+        subscription.getTime().setCycleTimes(
+                Arrays.asList(Integer.valueOf(9), Integer.valueOf(0)));
+
+        TreeSet<Integer> cycles = new TreeSet<Integer>(subscription.getTime()
+                .getCycleTimes());
+
+        SortedSet<Calendar> subscriptionTimes = new TreeSet<Calendar>();
+
+        subscriptionTimes = bandwidthDaoUtil.getRetrievalTimes(subscription,
+                cycles, plan, subscriptionTimes);
+
+        final List<Integer> daysOfTheYear = Arrays.asList(4);
+        verifySubscriptionTimesContainsCyclesForSpecifiedDays(daysOfTheYear,
+                cycles, subscriptionTimes);
+        final List<Integer> notScheduledDays = Arrays.asList(3);
+        verifySubscriptionTimesDoesNotContainCyclesForSpecifiedDays(
+                notScheduledDays, cycles, subscriptionTimes);
+    }
+
+    @Test
+    public void testGetRetrievalTimesUsesSubscriptionEndWhenItIsMostRestrictive() {
+        final Date endsOneDayBeforePlan = new Date(plan.getPlanEnd()
+                .getTimeInMillis() - TimeUtil.MILLIS_PER_DAY);
+
+        Subscription subscription = new SubscriptionBuilder()
+                .withSubscriptionStart(plan.getPlanStart().getTime())
+                .withSubscriptionEnd(endsOneDayBeforePlan).build();
+        subscription.getTime().setCycleTimes(
+                Arrays.asList(Integer.valueOf(9), Integer.valueOf(0)));
+
+        TreeSet<Integer> cycles = new TreeSet<Integer>(subscription.getTime()
+                .getCycleTimes());
+
+        SortedSet<Calendar> subscriptionTimes = new TreeSet<Calendar>();
+
+        subscriptionTimes = bandwidthDaoUtil.getRetrievalTimes(subscription,
+                cycles, plan, subscriptionTimes);
+
+        final List<Integer> daysOfTheYear = Arrays.asList(3);
+        verifySubscriptionTimesContainsCyclesForSpecifiedDays(daysOfTheYear,
+                cycles, subscriptionTimes);
+        final List<Integer> notScheduledDays = Arrays.asList(4);
+        verifySubscriptionTimesDoesNotContainCyclesForSpecifiedDays(
+                notScheduledDays, cycles, subscriptionTimes);
     }
 
     @Test
@@ -158,4 +206,58 @@ public class BandwidthDaoUtilTest {
         verify(retrievalManager, never()).remove(alloc2);
     }
 
+    /**
+     * Verifies the subscription times contains the cycles for the specified
+     * days.
+     * 
+     * @param daysOfTheYear
+     * @param cycles
+     * @param subscriptionTimes
+     */
+    private void verifySubscriptionTimesContainsCyclesForSpecifiedDays(
+            Collection<Integer> daysOfTheYear, Collection<Integer> cycles,
+            SortedSet<Calendar> subscriptionTimes) {
+        Calendar cal = TimeUtil.newCalendar();
+        TimeUtil.minCalendarFields(cal, Calendar.MILLISECOND, Calendar.SECOND,
+                Calendar.MINUTE);
+
+        for (Integer dayOfTheYear : daysOfTheYear) {
+            cal.set(Calendar.DAY_OF_YEAR, dayOfTheYear);
+
+            for (Integer cycle : cycles) {
+                cal.set(Calendar.HOUR_OF_DAY, cycle);
+                assertTrue("Expected to find retrieval time of "
+                        + BandwidthUtil.format(cal),
+                        subscriptionTimes.contains(cal));
+            }
+        }
+    }
+
+    /**
+     * Verifies the subscription times do not contain the cycles for the
+     * specified days.
+     * 
+     * @param daysOfTheYear
+     * @param cycles
+     * @param subscriptionTimes
+     */
+    private void verifySubscriptionTimesDoesNotContainCyclesForSpecifiedDays(
+            Collection<Integer> daysOfTheYear, Collection<Integer> cycles,
+            SortedSet<Calendar> subscriptionTimes) {
+        Calendar cal = TimeUtil.newCalendar();
+        TimeUtil.minCalendarFields(cal, Calendar.MILLISECOND, Calendar.SECOND,
+                Calendar.MINUTE);
+
+        for (Integer dayOfTheYear : daysOfTheYear) {
+            cal.set(Calendar.DAY_OF_YEAR, dayOfTheYear);
+
+            for (Integer cycle : cycles) {
+                cal.set(Calendar.HOUR_OF_DAY, cycle);
+                assertFalse("Expected not to find retrieval time of "
+                        + BandwidthUtil.format(cal),
+                        subscriptionTimes.contains(cal));
+            }
+        }
+
+    }
 }