Committed by
Gerrit Code Review
Change the behavior of resource registration to fix ONOS-3827
Treat as a failure when ID is found but the value is not found Change-Id: I032d25885897d662e49223a7b506eda8e5550a36
Showing
1 changed file
with
15 additions
and
10 deletions
| ... | @@ -19,6 +19,7 @@ import com.google.common.annotations.Beta; | ... | @@ -19,6 +19,7 @@ import com.google.common.annotations.Beta; |
| 19 | import com.google.common.collect.ImmutableList; | 19 | import com.google.common.collect.ImmutableList; |
| 20 | import com.google.common.collect.ImmutableSet; | 20 | import com.google.common.collect.ImmutableSet; |
| 21 | import com.google.common.collect.Maps; | 21 | import com.google.common.collect.Maps; |
| 22 | +import com.google.common.collect.Sets; | ||
| 22 | import org.apache.felix.scr.annotations.Activate; | 23 | import org.apache.felix.scr.annotations.Activate; |
| 23 | import org.apache.felix.scr.annotations.Component; | 24 | import org.apache.felix.scr.annotations.Component; |
| 24 | import org.apache.felix.scr.annotations.Reference; | 25 | import org.apache.felix.scr.annotations.Reference; |
| ... | @@ -507,25 +508,29 @@ public class ConsistentResourceStore extends AbstractStore<ResourceEvent, Resour | ... | @@ -507,25 +508,29 @@ public class ConsistentResourceStore extends AbstractStore<ResourceEvent, Resour |
| 507 | // computational complexity: O(n) where n is the number of the specified value | 508 | // computational complexity: O(n) where n is the number of the specified value |
| 508 | private boolean appendValues(TransactionalMap<DiscreteResourceId, Set<Resource>> map, | 509 | private boolean appendValues(TransactionalMap<DiscreteResourceId, Set<Resource>> map, |
| 509 | DiscreteResourceId key, List<Resource> values) { | 510 | DiscreteResourceId key, List<Resource> values) { |
| 510 | - Set<Resource> oldValues = map.putIfAbsent(key, new LinkedHashSet<>(values)); | 511 | + Set<Resource> requested = new LinkedHashSet<>(values); |
| 512 | + Set<Resource> oldValues = map.putIfAbsent(key, requested); | ||
| 511 | if (oldValues == null) { | 513 | if (oldValues == null) { |
| 512 | return true; | 514 | return true; |
| 513 | } | 515 | } |
| 514 | 516 | ||
| 515 | - Set<ResourceId> oldIds = oldValues.stream() | 517 | + Set<Resource> addedValues = Sets.difference(requested, oldValues); |
| 516 | - .map(Resource::id) | 518 | + // no new value, then no-op |
| 517 | - .collect(Collectors.toSet()); | ||
| 518 | - // values whose IDs don't match any IDs of oldValues | ||
| 519 | - Set<Resource> addedValues = values.stream() | ||
| 520 | - .filter(x -> !oldIds.contains(x.id())) | ||
| 521 | - .collect(Collectors.toCollection(LinkedHashSet::new)); | ||
| 522 | - // no new ID, then no-op | ||
| 523 | if (addedValues.isEmpty()) { | 519 | if (addedValues.isEmpty()) { |
| 524 | // don't write to map because all values are already stored | 520 | // don't write to map because all values are already stored |
| 525 | return true; | 521 | return true; |
| 526 | } | 522 | } |
| 527 | 523 | ||
| 528 | - LinkedHashSet<Resource> newValues = new LinkedHashSet<>(oldValues); | 524 | + Set<ResourceId> addedIds = addedValues.stream() |
| 525 | + .map(Resource::id) | ||
| 526 | + .collect(Collectors.toSet()); | ||
| 527 | + // if the value is not found but the same ID is found | ||
| 528 | + // (this happens only when being continuous resource) | ||
| 529 | + if (oldValues.stream().anyMatch(x -> addedIds.contains(x.id()))) { | ||
| 530 | + // no-op, but indicating failure (reject the request) | ||
| 531 | + return false; | ||
| 532 | + } | ||
| 533 | + Set<Resource> newValues = new LinkedHashSet<>(oldValues); | ||
| 529 | newValues.addAll(addedValues); | 534 | newValues.addAll(addedValues); |
| 530 | return map.replace(key, oldValues, newValues); | 535 | return map.replace(key, oldValues, newValues); |
| 531 | } | 536 | } | ... | ... |
-
Please register or login to post a comment