Committed by
Brian O'Connor
Return null instead of throw exception for invalid event states.
Change-Id: Ie40ca4dc1c241aa4f27652aa4e8b3f99eb924169
Showing
3 changed files
with
28 additions
and
25 deletions
| ... | @@ -72,10 +72,26 @@ public class IntentEvent extends AbstractEvent<IntentEvent.Type, Intent> { | ... | @@ -72,10 +72,26 @@ public class IntentEvent extends AbstractEvent<IntentEvent.Type, Intent> { |
| 72 | super(type, intent); | 72 | super(type, intent); |
| 73 | } | 73 | } |
| 74 | 74 | ||
| 75 | + /** | ||
| 76 | + * Creates an IntentEvent based on the state contained in the given intent | ||
| 77 | + * data. Some states are not sent as external events, and these states will | ||
| 78 | + * return null events. | ||
| 79 | + * | ||
| 80 | + * @param data the intent data to create an event for | ||
| 81 | + * @return new intent event if the state is valid, otherwise null. | ||
| 82 | + */ | ||
| 75 | public static IntentEvent getEvent(IntentData data) { | 83 | public static IntentEvent getEvent(IntentData data) { |
| 76 | return getEvent(data.state(), data.intent()); | 84 | return getEvent(data.state(), data.intent()); |
| 77 | } | 85 | } |
| 78 | 86 | ||
| 87 | + /** | ||
| 88 | + * Creates an IntentEvent based on the given state and intent. Some states | ||
| 89 | + * are not sent as external events, and these states will return null events. | ||
| 90 | + * | ||
| 91 | + * @param state new state of the intent | ||
| 92 | + * @param intent intent to put in event | ||
| 93 | + * @return new intent event if the state is valid, otherwise null. | ||
| 94 | + */ | ||
| 79 | public static IntentEvent getEvent(IntentState state, Intent intent) { | 95 | public static IntentEvent getEvent(IntentState state, Intent intent) { |
| 80 | Type type; | 96 | Type type; |
| 81 | switch (state) { | 97 | switch (state) { |
| ... | @@ -95,14 +111,13 @@ public class IntentEvent extends AbstractEvent<IntentEvent.Type, Intent> { | ... | @@ -95,14 +111,13 @@ public class IntentEvent extends AbstractEvent<IntentEvent.Type, Intent> { |
| 95 | type = Type.FAILED; | 111 | type = Type.FAILED; |
| 96 | break; | 112 | break; |
| 97 | 113 | ||
| 98 | - //fallthrough to default from here | 114 | + // fallthrough to default from here |
| 99 | case COMPILING: | 115 | case COMPILING: |
| 100 | case INSTALLING: | 116 | case INSTALLING: |
| 101 | case RECOMPILING: | 117 | case RECOMPILING: |
| 102 | case WITHDRAWING: | 118 | case WITHDRAWING: |
| 103 | default: | 119 | default: |
| 104 | - throw new IllegalArgumentException( | 120 | + return null; |
| 105 | - "Intent event cannot have transient state: " + state); | ||
| 106 | } | 121 | } |
| 107 | return new IntentEvent(type, intent); | 122 | return new IntentEvent(type, intent); |
| 108 | } | 123 | } | ... | ... |
| ... | @@ -268,9 +268,8 @@ public class GossipIntentStore | ... | @@ -268,9 +268,8 @@ public class GossipIntentStore |
| 268 | 268 | ||
| 269 | @Override | 269 | @Override |
| 270 | public void addPending(IntentData data) { | 270 | public void addPending(IntentData data) { |
| 271 | - log.debug("new call to pending {}", data); | 271 | + log.debug("new pending {} {} {}", data.key(), data.state(), data.version()); |
| 272 | if (data.version() == null) { | 272 | if (data.version() == null) { |
| 273 | - log.debug("updating timestamp"); | ||
| 274 | data.setVersion(new SystemClockTimestamp()); | 273 | data.setVersion(new SystemClockTimestamp()); |
| 275 | } | 274 | } |
| 276 | pending.put(data.key(), copyData(data)); | 275 | pending.put(data.key(), copyData(data)); |
| ... | @@ -293,16 +292,9 @@ public class GossipIntentStore | ... | @@ -293,16 +292,9 @@ public class GossipIntentStore |
| 293 | public void event( | 292 | public void event( |
| 294 | EventuallyConsistentMapEvent<Key, IntentData> event) { | 293 | EventuallyConsistentMapEvent<Key, IntentData> event) { |
| 295 | if (event.type() == EventuallyConsistentMapEvent.Type.PUT) { | 294 | if (event.type() == EventuallyConsistentMapEvent.Type.PUT) { |
| 296 | - IntentEvent externalEvent; | ||
| 297 | IntentData intentData = event.value(); | 295 | IntentData intentData = event.value(); |
| 298 | 296 | ||
| 299 | - try { | 297 | + notifyDelegateIfNotNull(IntentEvent.getEvent(intentData)); |
| 300 | - externalEvent = IntentEvent.getEvent(intentData.state(), intentData.intent()); | ||
| 301 | - } catch (IllegalArgumentException e) { | ||
| 302 | - externalEvent = null; | ||
| 303 | - } | ||
| 304 | - | ||
| 305 | - notifyDelegateIfNotNull(externalEvent); | ||
| 306 | } | 298 | } |
| 307 | } | 299 | } |
| 308 | } | 300 | } |
| ... | @@ -322,12 +314,7 @@ public class GossipIntentStore | ... | @@ -322,12 +314,7 @@ public class GossipIntentStore |
| 322 | } | 314 | } |
| 323 | } | 315 | } |
| 324 | 316 | ||
| 325 | - try { | 317 | + notifyDelegateIfNotNull(IntentEvent.getEvent(event.value())); |
| 326 | - notifyDelegate(IntentEvent.getEvent(event.value())); | ||
| 327 | - } catch (IllegalArgumentException e) { | ||
| 328 | - //no-op | ||
| 329 | - log.trace("ignore this exception: {}", e); | ||
| 330 | - } | ||
| 331 | } | 318 | } |
| 332 | } | 319 | } |
| 333 | } | 320 | } | ... | ... |
| ... | @@ -198,14 +198,15 @@ public class SimpleIntentStore | ... | @@ -198,14 +198,15 @@ public class SimpleIntentStore |
| 198 | pending.remove(newData.key()); | 198 | pending.remove(newData.key()); |
| 199 | } | 199 | } |
| 200 | 200 | ||
| 201 | - try { | 201 | + notifyDelegateIfNotNull(IntentEvent.getEvent(newData)); |
| 202 | - notifyDelegate(IntentEvent.getEvent(newData)); | ||
| 203 | - } catch (IllegalArgumentException e) { | ||
| 204 | - //no-op | ||
| 205 | - log.trace("ignore this exception: {}", e); | ||
| 206 | } | 202 | } |
| 207 | } | 203 | } |
| 208 | } | 204 | } |
| 205 | + | ||
| 206 | + private void notifyDelegateIfNotNull(IntentEvent event) { | ||
| 207 | + if (event != null) { | ||
| 208 | + notifyDelegate(event); | ||
| 209 | + } | ||
| 209 | } | 210 | } |
| 210 | 211 | ||
| 211 | @Override | 212 | @Override |
| ... | @@ -241,7 +242,7 @@ public class SimpleIntentStore | ... | @@ -241,7 +242,7 @@ public class SimpleIntentStore |
| 241 | pending.put(data.key(), data); | 242 | pending.put(data.key(), data); |
| 242 | checkNotNull(delegate, "Store delegate is not set") | 243 | checkNotNull(delegate, "Store delegate is not set") |
| 243 | .process(data); | 244 | .process(data); |
| 244 | - notifyDelegate(IntentEvent.getEvent(data)); | 245 | + notifyDelegateIfNotNull(IntentEvent.getEvent(data)); |
| 245 | } else { | 246 | } else { |
| 246 | log.debug("IntentData {} is older than existing: {}", | 247 | log.debug("IntentData {} is older than existing: {}", |
| 247 | data, existingData); | 248 | data, existingData); | ... | ... |
-
Please register or login to post a comment