From 6b46be5bc6f737b038c01327a795da4ace1bd3b9 Mon Sep 17 00:00:00 2001 From: Dustin Johnson Date: Mon, 28 Jan 2013 10:43:11 -0600 Subject: [PATCH] Issue #1530 Do not allow partial scheduling of subscriptions in BandwidthManager Amend: Rebase to 13.2.1 Change-Id: Ied72baede3691957cf1155e1986428d1bfd16e81 Former-commit-id: b0125d6d7ec2ad94f8dc4279a68fc252b0b721a0 [formerly b0125d6d7ec2ad94f8dc4279a68fc252b0b721a0 [formerly 902feab2b1bdf0ffdadce76847ee76b9a8c7061e]] Former-commit-id: 4e9ecdf166a4db55912171b04afbf32e8b43866d Former-commit-id: 150c5275ca2d944a7abae4b2bf21cd7d8226adbb --- .../subscription/SubscriptionService.java | 4 ++ .../subscription/subset/SubsetManagerDlg.java | 10 ++++- .../common/datadelivery/registry/Levels.java | 3 +- .../bandwidth/BandwidthManager.java | 44 +++++++++++++++++-- .../bandwidth/BandwidthManagerIntTest.java | 42 +++++++++++++++--- 5 files changed, 92 insertions(+), 11 deletions(-) diff --git a/cave/com.raytheon.uf.viz.datadelivery/src/com/raytheon/uf/viz/datadelivery/subscription/SubscriptionService.java b/cave/com.raytheon.uf.viz.datadelivery/src/com/raytheon/uf/viz/datadelivery/subscription/SubscriptionService.java index 8fc0783c17..80d5564a59 100644 --- a/cave/com.raytheon.uf.viz.datadelivery/src/com/raytheon/uf/viz/datadelivery/subscription/SubscriptionService.java +++ b/cave/com.raytheon.uf.viz.datadelivery/src/com/raytheon/uf/viz/datadelivery/subscription/SubscriptionService.java @@ -71,6 +71,7 @@ import com.raytheon.uf.viz.datadelivery.utils.DataDeliveryUtils; * Dec 18, 2012 1443 bgonzale Open force apply prompt pop-up on the UI thread. * Dec 20, 2012 1413 bgonzale Added new pending approve and denied request and responses. * Jan 04, 2013 1441 djohnson Separated out notification methods into their own service. + * Jan 28, 2013 1530 djohnson Reset unscheduled flag with each update. * * * @@ -358,6 +359,7 @@ public class SubscriptionService implements ISubscriptionService { final ServiceInteraction action = new ServiceInteraction() { @Override public String call() throws RegistryHandlerException { + subscription.setUnscheduled(false); DataDeliveryHandlers.getSubscriptionHandler().update( subscription); return successMessage; @@ -380,6 +382,7 @@ public class SubscriptionService implements ISubscriptionService { @Override public String call() throws RegistryHandlerException { for (Subscription sub : subs) { + sub.setUnscheduled(false); DataDeliveryHandlers.getSubscriptionHandler().update(sub); } return successMessage; @@ -439,6 +442,7 @@ public class SubscriptionService implements ISubscriptionService { subscription).isAuthorized(); try { if (authorized) { + subscription.setUnscheduled(false); DataDeliveryHandlers.getSubscriptionHandler() .update(subscription); } else { diff --git a/cave/com.raytheon.uf.viz.datadelivery/src/com/raytheon/uf/viz/datadelivery/subscription/subset/SubsetManagerDlg.java b/cave/com.raytheon.uf.viz.datadelivery/src/com/raytheon/uf/viz/datadelivery/subscription/subset/SubsetManagerDlg.java index 6fa341661d..f4040cc45f 100644 --- a/cave/com.raytheon.uf.viz.datadelivery/src/com/raytheon/uf/viz/datadelivery/subscription/subset/SubsetManagerDlg.java +++ b/cave/com.raytheon.uf.viz.datadelivery/src/com/raytheon/uf/viz/datadelivery/subscription/subset/SubsetManagerDlg.java @@ -22,6 +22,7 @@ package com.raytheon.uf.viz.datadelivery.subscription.subset; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; @@ -51,6 +52,7 @@ import com.raytheon.uf.common.datadelivery.registry.DataSet; import com.raytheon.uf.common.datadelivery.registry.DataType; import com.raytheon.uf.common.datadelivery.registry.GriddedCoverage; import com.raytheon.uf.common.datadelivery.registry.GriddedDataSet; +import com.raytheon.uf.common.datadelivery.registry.Levels; import com.raytheon.uf.common.datadelivery.registry.Parameter; import com.raytheon.uf.common.datadelivery.registry.Subscription; import com.raytheon.uf.common.datadelivery.registry.Time; @@ -123,6 +125,7 @@ import com.raytheon.viz.ui.presenter.IDisplay; * Jan 02, 2012 1345 djohnson Use gui thread task executor. * Jan 04, 2012 1420 mpduff Pass the subscription in to the GriddedTimingSelectionDlg. * Jan 10, 2013 1444 mpduff Fix the loading of saved subsets from the saved subset tab. + * Jan 28, 2013 1530 djohnson Break out long method chaining into local variables for debugging. * * * @author mpduff @@ -907,8 +910,11 @@ public abstract class SubsetManagerDlg selectedLevelIndices = levels + .getSelectedLevelIndices(); + for (int index : selectedLevelIndices) { + v.addLevel(String.valueOf(levels.getLevel() .get(index))); } } diff --git a/edexOsgi/com.raytheon.uf.common.datadelivery.registry/src/com/raytheon/uf/common/datadelivery/registry/Levels.java b/edexOsgi/com.raytheon.uf.common.datadelivery.registry/src/com/raytheon/uf/common/datadelivery/registry/Levels.java index 4407a8b0f2..a8eec4aa03 100644 --- a/edexOsgi/com.raytheon.uf.common.datadelivery.registry/src/com/raytheon/uf/common/datadelivery/registry/Levels.java +++ b/edexOsgi/com.raytheon.uf.common.datadelivery.registry/src/com/raytheon/uf/common/datadelivery/registry/Levels.java @@ -25,6 +25,7 @@ import com.raytheon.uf.common.serialization.annotations.DynamicSerializeElement; * Feb 08, 2011 191 dhladky Initial creation * Jul 24, 2012 955 djohnson Use List instead of ArrayList. * Nov 19, 2012 1166 djohnson Clean up JAXB representation of registry objects. + * Jan 28, 2013 1530 djohnson Never return null for selected level indices. * * * @author dhladky @@ -67,7 +68,7 @@ public class Levels implements ISerializableObject, Serializable { @XmlElements({ @XmlElement(name = "selectedLevelIndices", type = Integer.class) }) @DynamicSerializeElement - private List selectedLevelIndices; + private List selectedLevelIndices = new ArrayList(); /** * Copy constructor diff --git a/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManager.java b/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManager.java index 29504e6ef6..7392b1b1c8 100644 --- a/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManager.java +++ b/edexOsgi/com.raytheon.uf.edex.datadelivery.bandwidth/src/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManager.java @@ -97,6 +97,7 @@ import com.raytheon.uf.edex.event.EventBus; * Dec 11, 2012 1403 djohnson Adhoc subscriptions no longer go to the registry. * Dec 12, 2012 1286 djohnson Remove shutdown hook and finalize(). * Jan 25, 2013 1528 djohnson Compare priorities as primitive ints. + * Jan 28, 2013 1530 djohnson Unschedule all allocations for a subscription that does not fully schedule. * * * @@ -568,9 +569,45 @@ abstract class BandwidthManager extends .getCycleTimes()); List unscheduled = schedule(subscription, cycles); + unscheduleSubscriptionsForAllocations(unscheduled); + return unscheduled; } + /** + * Unschedules all subscriptions the allocations are associated to. + * + * @param unscheduled + * the unscheduled allocations + */ + private void unscheduleSubscriptionsForAllocations( + List unscheduled) { + Set subscriptionsToUnschedule = Sets.newHashSet(); + for (BandwidthAllocation unscheduledAllocation : unscheduled) { + if (unscheduledAllocation instanceof SubscriptionRetrieval) { + SubscriptionRetrieval retrieval = (SubscriptionRetrieval) unscheduledAllocation; + try { + subscriptionsToUnschedule.add(retrieval.getSubscription()); + } catch (SerializationException e) { + statusHandler.handle(Priority.PROBLEM, + "Unable to deserialize a subscription", e); + continue; + } + } + } + + for (Subscription sub : subscriptionsToUnschedule) { + sub.setUnscheduled(true); + try { + subscriptionUpdated(sub); + } catch (SerializationException e) { + statusHandler.handle(Priority.PROBLEM, + "Unable to deserialize a subscription", e); + continue; + } + } + } + /** * {@inheritDoc} * @@ -659,7 +696,8 @@ abstract class BandwidthManager extends // If BandwidthManager does not know about the subscription, and // it's active, attempt to add it.. - if (subscriptionDaos.isEmpty() && subscription.isActive()) { + if (subscriptionDaos.isEmpty() && subscription.isActive() + && !subscription.isUnscheduled()) { final boolean subscribedToCycles = !subscription.getTime() .getCycleTimes().isEmpty(); final boolean useMostRecentDataSetUpdate = !subscribedToCycles; @@ -688,8 +726,8 @@ abstract class BandwidthManager extends unscheduled = schedule(adhoc); } return unscheduled; - } else if (!subscription.isActive()) { - // See if the subscription was inactivated.. + } else if (!subscription.isActive() || subscription.isUnscheduled()) { + // See if the subscription was inactivated or unscheduled.. // Need to remove BandwidthReservations for this // subscription. return remove(subscriptionDaos, true); diff --git a/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManagerIntTest.java b/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManagerIntTest.java index 93e0e507ba..d7b7c507d1 100644 --- a/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManagerIntTest.java +++ b/tests/integration/com/raytheon/uf/edex/datadelivery/bandwidth/BandwidthManagerIntTest.java @@ -19,8 +19,11 @@ **/ package com.raytheon.uf.edex.datadelivery.bandwidth; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import java.text.ParseException; @@ -83,6 +86,7 @@ import com.raytheon.uf.edex.datadelivery.retrieval.RetrievalManagerNotifyEvent; * Oct 23, 2012 1286 djohnson Create reusable abstract int test. * Dec 11, 2012 1286 djohnson Add test verifying fulfilled retrievals won't cause NPEs when the subscription is updated. * 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. * * * @@ -347,6 +351,35 @@ public class BandwidthManagerIntTest extends AbstractBandwidthManagerIntTest { unscheduledAllocation.getPriority(), 0.0); } + @Test + public void unscheduledSubscriptionUnschedulesAllAllocations() { + String unscheduledSubDataSetName = "willBeUnscheduled"; + Subscription subscription = createSubscriptionThatFillsUpABucket(); + subscription.setDataSetName(unscheduledSubDataSetName); + Subscription subscription2 = createSubscriptionThatFillsUpABucket(); + + // subscription2 will have higher priority + subscription2.setPriority(SubscriptionPriority.HIGH); + + // they conflict for cycle hour 8 + subscription.getTime().setCycleTimes( + Arrays.asList(Integer.valueOf(6), Integer.valueOf(8))); + subscription2.getTime().setCycleTimes( + Arrays.asList(Integer.valueOf(3), Integer.valueOf(8))); + + bandwidthManager.schedule(subscription); + bandwidthManager.schedule(subscription2); + + final List subscriptionRetrievals = bandwidthDao + .getSubscriptionRetrievals(subscription.getProvider(), + unscheduledSubDataSetName); + + for (SubscriptionRetrieval subscriptionRetrieval : subscriptionRetrievals) { + assertThat(subscriptionRetrieval.getStatus(), + is(equalTo(RetrievalStatus.UNSCHEDULED))); + } + } + @Test public void testScheduleSubscriptionWithHigherPrioritySetsOtherAllocationsToUnscheduled() { @@ -421,8 +454,6 @@ public class BandwidthManagerIntTest extends AbstractBandwidthManagerIntTest { // Subscription starts out too big Subscription subscription = createSubscriptionThatFillsUpTwoBuckets(); - - // they conflict for cycle hour 8 subscription.getTime().setCycleTimes(Arrays.asList(Integer.valueOf(6))); List unscheduled = bandwidthManager @@ -434,6 +465,7 @@ public class BandwidthManagerIntTest extends AbstractBandwidthManagerIntTest { // Hey look, this subscription will fit now! subscription.setDataSetSize(subscription.getDataSetSize() / 2); + subscription.setUnscheduled(false); unscheduled = bandwidthManager.subscriptionUpdated(subscription); @@ -466,7 +498,7 @@ public class BandwidthManagerIntTest extends AbstractBandwidthManagerIntTest { int requiredLatency = bandwidthManager .determineRequiredLatency(subscription); - assertEquals("The required latency was calculated incorrectly", 6, + assertEquals("The required latency was calculated incorrectly", 7, requiredLatency); } @@ -714,7 +746,7 @@ public class BandwidthManagerIntTest extends AbstractBandwidthManagerIntTest { final List bandwidthAllocations = bandwidthDao .getBandwidthAllocations(subscription.getRoute()); - assertEquals("Incorrect number of allocations found.", 4, + assertEquals("Incorrect number of allocations found.", 0, bandwidthAllocations.size()); sendDeletedSubscriptionEvent(subscription); @@ -747,7 +779,7 @@ public class BandwidthManagerIntTest extends AbstractBandwidthManagerIntTest { final List subscriptionDaos = bandwidthDao .getSubscriptionDao(subscription); - assertEquals("Incorrect number of subscription daos found.", 4, + assertEquals("Incorrect number of subscription daos found.", 0, subscriptionDaos.size()); sendDeletedSubscriptionEvent(subscription);