Sho SHIMIZU

Rename arguments to be more descriptive

Change-Id: If5c51aa2d713414839650e689ed8c4050a53c3a2
......@@ -22,8 +22,8 @@ import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Reference;
import org.apache.felix.scr.annotations.ReferenceCardinality;
import org.apache.felix.scr.annotations.Service;
import org.onlab.util.Tools;
import org.onlab.util.KryoNamespace;
import org.onlab.util.Tools;
import org.onosproject.net.resource.ContinuousResource;
import org.onosproject.net.resource.ContinuousResourceId;
import org.onosproject.net.resource.DiscreteResource;
......@@ -334,28 +334,28 @@ public class ConsistentResourceStore extends AbstractStore<ResourceEvent, Resour
* Appends the values to the existing values associated with the specified key.
* If the map already has all the given values, appending will not happen.
*
* @param key key specifying values
* @param values values to be appended
* @param parent resource ID of the parent under which the given resources are registered
* @param resources resources to be registered
* @return true if the operation succeeds, false otherwise.
*/
// computational complexity: O(n) where n is the number of the specified value
private boolean register(TransactionalDiscreteResourceSubStore discreteTxStore,
TransactionalContinuousResourceSubStore continuousTxStore,
DiscreteResourceId key, List<Resource> values) {
DiscreteResourceId parent, List<Resource> resources) {
// it's assumed that the passed "values" is non-empty
// This is 2-pass scan. Nicer to have 1-pass scan
Set<DiscreteResource> discreteValues = values.stream()
Set<DiscreteResource> discreteResources = resources.stream()
.filter(x -> x instanceof DiscreteResource)
.map(x -> (DiscreteResource) x)
.collect(Collectors.toCollection(LinkedHashSet::new));
Set<ContinuousResource> continuousValues = values.stream()
Set<ContinuousResource> continuousResources = resources.stream()
.filter(x -> x instanceof ContinuousResource)
.map(x -> (ContinuousResource) x)
.collect(Collectors.toCollection(LinkedHashSet::new));
return discreteTxStore.register(key, discreteValues)
&& continuousTxStore.register(key, continuousValues);
return discreteTxStore.register(parent, discreteResources)
&& continuousTxStore.register(parent, continuousResources);
}
/**
......@@ -364,26 +364,26 @@ public class ConsistentResourceStore extends AbstractStore<ResourceEvent, Resour
*
* @param discreteTxStore map holding multiple discrete resources for a key
* @param continuousTxStore map holding multiple continuous resources for a key
* @param key key specifying values
* @param values values to be removed
* @param parent resource ID of the parent under which the given resources are unregistered
* @param resources resources to be unregistered
* @return true if the operation succeeds, false otherwise
*/
private boolean unregister(TransactionalDiscreteResourceSubStore discreteTxStore,
TransactionalContinuousResourceSubStore continuousTxStore,
DiscreteResourceId key, List<Resource> values) {
DiscreteResourceId parent, List<Resource> resources) {
// it's assumed that the passed "values" is non-empty
// This is 2-pass scan. Nicer to have 1-pass scan
Set<DiscreteResource> discreteValues = values.stream()
Set<DiscreteResource> discreteResources = resources.stream()
.filter(x -> x instanceof DiscreteResource)
.map(x -> (DiscreteResource) x)
.collect(Collectors.toCollection(LinkedHashSet::new));
Set<ContinuousResource> continuousValues = values.stream()
Set<ContinuousResource> continuousResources = resources.stream()
.filter(x -> x instanceof ContinuousResource)
.map(x -> (ContinuousResource) x)
.collect(Collectors.toCollection(LinkedHashSet::new));
return discreteTxStore.unregister(key, discreteValues)
&& continuousTxStore.unregister(key, continuousValues);
return discreteTxStore.unregister(parent, discreteResources)
&& continuousTxStore.unregister(parent, continuousResources);
}
}
......
......@@ -60,18 +60,18 @@ class TransactionalContinuousResourceSubStore {
.findFirst();
}
boolean register(DiscreteResourceId key, Set<ContinuousResource> requested) {
boolean register(DiscreteResourceId parent, Set<ContinuousResource> resources) {
// short-circuit: receiving empty resource is regarded as success
if (requested.isEmpty()) {
if (resources.isEmpty()) {
return true;
}
Set<ContinuousResource> oldValues = childMap.putIfAbsent(key, requested);
Set<ContinuousResource> oldValues = childMap.putIfAbsent(parent, resources);
if (oldValues == null) {
return true;
}
Set<ContinuousResource> addedValues = Sets.difference(requested, oldValues);
Set<ContinuousResource> addedValues = Sets.difference(resources, oldValues);
// no new value, then no-op
if (addedValues.isEmpty()) {
// don't write to map because all values are already stored
......@@ -89,38 +89,38 @@ class TransactionalContinuousResourceSubStore {
}
Set<ContinuousResource> newValues = new LinkedHashSet<>(oldValues);
newValues.addAll(addedValues);
return childMap.replace(key, oldValues, newValues);
return childMap.replace(parent, oldValues, newValues);
}
boolean unregister(DiscreteResourceId key, Set<ContinuousResource> values) {
boolean unregister(DiscreteResourceId parent, Set<ContinuousResource> resources) {
// short-circuit: receiving empty resource is regarded as success
if (values.isEmpty()) {
if (resources.isEmpty()) {
return true;
}
// even if one of the resources is allocated to a consumer,
// all unregistrations are regarded as failure
boolean allocated = values.stream().anyMatch(x -> isAllocated(x.id()));
boolean allocated = resources.stream().anyMatch(x -> isAllocated(x.id()));
if (allocated) {
log.warn("Failed to unregister {}: allocation exists", key);
log.warn("Failed to unregister {}: allocation exists", parent);
return false;
}
Set<ContinuousResource> oldValues = childMap.putIfAbsent(key, new LinkedHashSet<>());
Set<ContinuousResource> oldValues = childMap.putIfAbsent(parent, new LinkedHashSet<>());
if (oldValues == null) {
log.trace("No-Op removing values. key {} did not exist", key);
log.trace("No-Op removing values. key {} did not exist", parent);
return true;
}
if (values.stream().allMatch(x -> !oldValues.contains(x))) {
if (resources.stream().allMatch(x -> !oldValues.contains(x))) {
// don't write map because none of the values are stored
log.trace("No-Op removing values. key {} did not contain {}", key, values);
log.trace("No-Op removing values. key {} did not contain {}", parent, resources);
return true;
}
LinkedHashSet<ContinuousResource> newValues = new LinkedHashSet<>(oldValues);
newValues.removeAll(values);
return childMap.replace(key, oldValues, newValues);
newValues.removeAll(resources);
return childMap.replace(parent, oldValues, newValues);
}
private boolean isAllocated(ContinuousResourceId id) {
......
......@@ -53,14 +53,14 @@ class TransactionalDiscreteResourceSubStore {
return values.lookup(id);
}
boolean register(DiscreteResourceId key, Set<DiscreteResource> values) {
boolean register(DiscreteResourceId parent, Set<DiscreteResource> resources) {
// short-circuit: receiving empty resource is regarded as success
if (values.isEmpty()) {
if (resources.isEmpty()) {
return true;
}
DiscreteResources requested = DiscreteResources.of(values);
DiscreteResources oldValues = childMap.putIfAbsent(key, requested);
DiscreteResources requested = DiscreteResources.of(resources);
DiscreteResources oldValues = childMap.putIfAbsent(parent, requested);
if (oldValues == null) {
return true;
}
......@@ -73,38 +73,38 @@ class TransactionalDiscreteResourceSubStore {
}
DiscreteResources newValues = oldValues.add(addedValues);
return childMap.replace(key, oldValues, newValues);
return childMap.replace(parent, oldValues, newValues);
}
boolean unregister(DiscreteResourceId key, Set<DiscreteResource> values) {
boolean unregister(DiscreteResourceId parent, Set<DiscreteResource> resources) {
// short-circuit: receiving empty resource is regarded as success
if (values.isEmpty()) {
if (resources.isEmpty()) {
return true;
}
// even if one of the resources is allocated to a consumer,
// all unregistrations are regarded as failure
boolean allocated = values.stream().anyMatch(x -> isAllocated(x.id()));
boolean allocated = resources.stream().anyMatch(x -> isAllocated(x.id()));
if (allocated) {
log.warn("Failed to unregister {}: allocation exists", key);
log.warn("Failed to unregister {}: allocation exists", parent);
return false;
}
DiscreteResources oldValues = childMap.putIfAbsent(key, DiscreteResources.empty());
DiscreteResources oldValues = childMap.putIfAbsent(parent, DiscreteResources.empty());
if (oldValues == null) {
log.trace("No-Op removing values. key {} did not exist", key);
log.trace("No-Op removing values. key {} did not exist", parent);
return true;
}
if (!oldValues.containsAny(values)) {
if (!oldValues.containsAny(resources)) {
// don't write map because none of the values are stored
log.trace("No-Op removing values. key {} did not contain {}", key, values);
log.trace("No-Op removing values. key {} did not contain {}", parent, resources);
return true;
}
DiscreteResources requested = DiscreteResources.of(values);
DiscreteResources requested = DiscreteResources.of(resources);
DiscreteResources newValues = oldValues.difference(requested);
return childMap.replace(key, oldValues, newValues);
return childMap.replace(parent, oldValues, newValues);
}
private boolean isAllocated(DiscreteResourceId id) {
......