ONOS-2446: Implement API to declare resource hierarchy

Remove API to define resource boundary
(ResourceAdminService.defineResourceBoundary) to integrate with API for
resource hierarchy

Change-Id: Iffa28dec16320122fe41f4f455000596fa266acb

@@ -17,7 +17,8 @@ package org.onosproject.net.newresource;
 
 
 import com.google.common.annotations.Beta;
 import com.google.common.annotations.Beta;
 
 
-import java.util.function.Predicate;
+import java.util.Arrays;
+import java.util.List;
 
 
 /**
 /**
  * Service for administering resource service behavior.
  * Service for administering resource service behavior.
@@ -25,13 +26,26 @@ import java.util.function.Predicate;
 @Beta
 @Beta
 public interface ResourceAdminService {
 public interface ResourceAdminService {
     /**
     /**
-     * Define a boundary of the resource specified by the class.
-     * The specified predicate is expected to return true if the supplied value is
-     * in the resource boundary and return false if it is out of the boundary.
+     * Register resources as the children of the parent resource path.
      *
      *
-     * @param cls class of the resource type
-     * @param predicate predicate returning true if the value is in the boundary
-     * @param <T> type of the resource
+     * @param parent parent resource path under which the resource are registered
+     * @param children resources to be registered as the children of the parent
+     * @param <T> type of resources
+     * @return true if registration is successfully done, false otherwise. Registration
+     * succeeds when each resource is not registered or unallocated.
      */
      */
-    <T> void defineResourceBoundary(Class<T> cls, Predicate<T> predicate);
+    default <T> boolean registerResources(ResourcePath parent, T... children) {
+        return registerResources(parent, Arrays.asList(children));
+    }
+
+    /**
+     * Register resources as the children of the parent resource path.
+     *
+     * @param parent parent resource path under which the resource are registered
+     * @param children resources to be registered as the children of the parent
+     * @param <T> type of resources
+     * @return true if registration is successfully done, false otherwise. Registration
+     * succeeds when each resource is not registered or unallocated.
+     */
+    <T> boolean registerResources(ResourcePath parent, List<T> children);
 }
 }

@@ -24,7 +24,6 @@ import java.util.List;
 import java.util.Objects;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Optional;
 
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 
 /**
 /**
@@ -42,6 +41,16 @@ public final class ResourcePath {
 
 
     private final List<Object> resources;
     private final List<Object> resources;
 
 
+    public static final ResourcePath ROOT = new ResourcePath(ImmutableList.of());
+
+    public static ResourcePath child(ResourcePath parent, Object child) {
+        ImmutableList<Object> components = ImmutableList.builder()
+                .addAll(parent.components())
+                .add(child)
+                .build();
+        return new ResourcePath(components);
+    }
+
     /**
     /**
      * Creates an resource path from the specified components.
      * Creates an resource path from the specified components.
      *
      *
@@ -58,7 +67,6 @@ public final class ResourcePath {
      */
      */
     public ResourcePath(List<Object> components) {
     public ResourcePath(List<Object> components) {
         checkNotNull(components);
         checkNotNull(components);
-        checkArgument(components.size() > 0);
 
 
         this.resources = ImmutableList.copyOf(components);
         this.resources = ImmutableList.copyOf(components);
     }
     }
@@ -85,7 +93,7 @@ public final class ResourcePath {
      * If there is no parent, empty instance will be returned.
      * If there is no parent, empty instance will be returned.
      */
      */
     public Optional<ResourcePath> parent() {
     public Optional<ResourcePath> parent() {
-        if (resources.size() >= 2) {
+        if (resources.size() > 0) {
             return Optional.of(new ResourcePath(resources.subList(0, resources.size() - 1)));
             return Optional.of(new ResourcePath(resources.subList(0, resources.size() - 1)));
         }
         }
 
 

@@ -11,6 +11,19 @@ import java.util.Optional;
  */
  */
 @Beta
 @Beta
 public interface ResourceStore {
 public interface ResourceStore {
+
+    /**
+     * Registers the resources as children of the parent resource in transactional way.
+     * Resource registration is must be done before resource allocation. The state after completion
+     * of this method is all the resources are registered, or no resource is registered.
+     * The whole registration fails when any one of the resource can't be registered.
+     *
+     * @param parent resource which is the parent of the resource to be registered
+     * @param children resources to be registered
+     * @return true if the registration succeeds, false otherwise
+     */
+    boolean register(ResourcePath parent, List<ResourcePath> children);
+
     /**
     /**
      * Allocates the specified resources to the specified consumer in transactional way.
      * Allocates the specified resources to the specified consumer in transactional way.
      * The state after completion of this method is all the resources are allocated to the consumer,
      * The state after completion of this method is all the resources are allocated to the consumer,

@@ -49,9 +49,11 @@ public class ResourcePathTest {
                 .testEquals();
                 .testEquals();
     }
     }
 
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void testCreateWithZeroComponent() {
     public void testCreateWithZeroComponent() {
         ResourcePath path = new ResourcePath();
         ResourcePath path = new ResourcePath();
+
+        assertThat(path, is(ResourcePath.ROOT));
     }
     }
 
 
     @Test
     @Test
@@ -66,7 +68,7 @@ public class ResourcePathTest {
     public void testNoParent() {
     public void testNoParent() {
         ResourcePath path = new ResourcePath(LinkKey.linkKey(CP1_1, CP2_1));
         ResourcePath path = new ResourcePath(LinkKey.linkKey(CP1_1, CP2_1));
 
 
-        assertThat(path.parent(), is(Optional.empty()));
+        assertThat(path.parent(), is(Optional.of(ResourcePath.ROOT)));
     }
     }
 
 
     @Test
     @Test

@@ -17,6 +17,7 @@ package org.onosproject.net.newresource.impl;
 
 
 import com.google.common.annotations.Beta;
 import com.google.common.annotations.Beta;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
@@ -33,9 +34,6 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collection;
 import java.util.List;
 import java.util.List;
 import java.util.Optional;
 import java.util.Optional;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Collectors;
 
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -48,8 +46,6 @@ import static com.google.common.base.Preconditions.checkNotNull;
 @Beta
 @Beta
 public final class ResourceManager implements ResourceService, ResourceAdminService {
 public final class ResourceManager implements ResourceService, ResourceAdminService {
 
 
-    private final ConcurrentMap<Class<?>, Predicate<?>> boundaries = new ConcurrentHashMap<>();
-
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected ResourceStore store;
     protected ResourceStore store;
 
 
@@ -79,10 +75,6 @@ public final class ResourceManager implements ResourceService, ResourceAdminServ
         checkNotNull(consumer);
         checkNotNull(consumer);
         checkNotNull(resources);
         checkNotNull(resources);
 
 
-        if (resources.stream().anyMatch(x -> !isValid(x))) {
-            return ImmutableList.of();
-        }
-
         // TODO: implement support of resource hierarchy
         // TODO: implement support of resource hierarchy
         // allocation for a particular resource implies allocations for all of the sub-resources need to be done
         // allocation for a particular resource implies allocations for all of the sub-resources need to be done
 
 
@@ -177,38 +169,8 @@ public final class ResourceManager implements ResourceService, ResourceAdminServ
     }
     }
 
 
     @Override
     @Override
-    public <T> void defineResourceBoundary(Class<T> cls, Predicate<T> predicate) {
-        boundaries.put(cls, predicate);
-    }
-
-    /**
-     * Returns the predicate associated with the specified resource.
-     *
-     * @param resource resource whose associated predicate is to be returned
-     * @param <T> type of the resource
-     * @return predicate associated with the resource
-     * Null if the resource doesn't have an associated predicate.
-     */
-    @SuppressWarnings("unchecked")
-    private <T> Predicate<T> lookupPredicate(T resource) {
-        return (Predicate<T>) boundaries.get(resource.getClass());
-    }
-
-    /**
-     * Returns if the specified resource is in the resource range.
-     * E.g. VLAN ID against a link must be within 12 bit address space.
-     *
-     * @param resource resource to be checked if it is within the resource range
-     * @return true if the resource within the range, false otherwise
-     */
-    boolean isValid(ResourcePath resource) {
-        List<Object> flatten = resource.components();
-        Object bottom = flatten.get(flatten.size() - 1);
-        Predicate<Object> predicate = lookupPredicate(bottom);
-        if (predicate == null) {
-            return true;
-        }
-
-        return predicate.test(bottom);
+    public <T> boolean registerResources(ResourcePath parent, List<T> children) {
+        List<ResourcePath> resources = Lists.transform(children, x -> ResourcePath.child(parent, x));
+        return store.register(parent, resources);
     }
     }
 }
 }

-/*
- * Copyright 2015 Open Networking Laboratory
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.onosproject.net.newresource.impl;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.onlab.packet.VlanId;
-import org.onosproject.net.ConnectPoint;
-import org.onosproject.net.DeviceId;
-import org.onosproject.net.LinkKey;
-import org.onosproject.net.PortNumber;
-import org.onosproject.net.newresource.ResourcePath;
-
-import java.util.function.Predicate;
-
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertThat;
-
-/**
- * Unit tests for ResourceManager.
- */
-public class ResourceManagerTest {
-
-    private static final DeviceId D1 = DeviceId.deviceId("of:001");
-    private static final DeviceId D2 = DeviceId.deviceId("of:002");
-    private static final PortNumber P1 = PortNumber.portNumber(1);
-    private static final ConnectPoint CP1_1 = new ConnectPoint(D1, P1);
-    private static final ConnectPoint CP2_1 = new ConnectPoint(D2, P1);
-    private static final short VLAN_LOWER_LIMIT = 0;
-    private static final short VLAN_UPPER_LIMIT = 1024;
-
-    private final Predicate<VlanId> vlanPredicate =
-            x -> x.toShort() >= VLAN_LOWER_LIMIT && x.toShort() < VLAN_UPPER_LIMIT;
-    private ResourceManager manager;
-
-    @Before
-    public void setUp() {
-        manager = new ResourceManager();
-    }
-
-    /**
-     * Tests resource boundaries.
-     */
-    @Test
-    public void testBoundary() {
-        manager.defineResourceBoundary(VlanId.class, vlanPredicate);
-
-        LinkKey linkKey = LinkKey.linkKey(CP1_1, CP2_1);
-
-        assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) (VLAN_LOWER_LIMIT - 1)))),
-                is(false));
-
-        assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId(VLAN_LOWER_LIMIT))),
-                is(true));
-
-        assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) 100))),
-                is(true));
-
-        assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) (VLAN_UPPER_LIMIT - 1)))),
-                is(true));
-
-        assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId(VLAN_UPPER_LIMIT))),
-                is(false));
-    }
-
-    /**
-     * Tests the case that a boundary is not set.
-     */
-    @Test
-    public void testWhenBoundaryNotSet() {
-        LinkKey linkKey = LinkKey.linkKey(CP1_1, CP2_1);
-
-        assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) (VLAN_LOWER_LIMIT - 1)))),
-                is(true));
-
-        assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId(VLAN_LOWER_LIMIT))),
-                is(true));
-
-        assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) 100))),
-                is(true));
-
-        assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId((short) (VLAN_UPPER_LIMIT - 1)))),
-                is(true));
-
-        assertThat(manager.isValid(new ResourcePath(linkKey, VlanId.vlanId(VLAN_UPPER_LIMIT))),
-                is(true));
-    }
-}

@@ -35,8 +35,12 @@ import org.onosproject.store.service.Versioned;
 import org.slf4j.Logger;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.LoggerFactory;
 
 
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.List;
 import java.util.Map;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Optional;
@@ -54,18 +58,25 @@ import static com.google.common.base.Preconditions.checkNotNull;
 public class ConsistentResourceStore implements ResourceStore {
 public class ConsistentResourceStore implements ResourceStore {
     private static final Logger log = LoggerFactory.getLogger(ConsistentResourceStore.class);
     private static final Logger log = LoggerFactory.getLogger(ConsistentResourceStore.class);
 
 
-    private static final String MAP_NAME = "onos-resource-consumers";
-    private static final Serializer SERIALIZER = Serializer.using(KryoNamespaces.API);
+    private static final String CONSUMER_MAP = "onos-resource-consumers";
+    private static final String CHILD_MAP = "onos-resource-children";
+    private static final Serializer SERIALIZER = Serializer.using(
+            Arrays.asList(KryoNamespaces.BASIC, KryoNamespaces.API));
 
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected StorageService service;
     protected StorageService service;
 
 
-    private ConsistentMap<ResourcePath, ResourceConsumer> consumers;
+    private ConsistentMap<ResourcePath, ResourceConsumer> consumerMap;
+    private ConsistentMap<ResourcePath, List<ResourcePath>> childMap;
 
 
     @Activate
     @Activate
     public void activate() {
     public void activate() {
-        consumers = service.<ResourcePath, ResourceConsumer>consistentMapBuilder()
-                .withName(MAP_NAME)
+        consumerMap = service.<ResourcePath, ResourceConsumer>consistentMapBuilder()
+                .withName(CONSUMER_MAP)
+                .withSerializer(SERIALIZER)
+                .build();
+        childMap = service.<ResourcePath, List<ResourcePath>>consistentMapBuilder()
+                .withName(CHILD_MAP)
                 .withSerializer(SERIALIZER)
                 .withSerializer(SERIALIZER)
                 .build();
                 .build();
     }
     }
@@ -74,7 +85,7 @@ public class ConsistentResourceStore implements ResourceStore {
     public Optional<ResourceConsumer> getConsumer(ResourcePath resource) {
     public Optional<ResourceConsumer> getConsumer(ResourcePath resource) {
         checkNotNull(resource);
         checkNotNull(resource);
 
 
-        Versioned<ResourceConsumer> consumer = consumers.get(resource);
+        Versioned<ResourceConsumer> consumer = consumerMap.get(resource);
         if (consumer == null) {
         if (consumer == null) {
             return Optional.empty();
             return Optional.empty();
         }
         }
@@ -83,6 +94,33 @@ public class ConsistentResourceStore implements ResourceStore {
     }
     }
 
 
     @Override
     @Override
+    public boolean register(ResourcePath resource, List<ResourcePath> children) {
+        checkNotNull(resource);
+        checkNotNull(children);
+
+        TransactionContext tx = service.transactionContextBuilder().build();
+        tx.begin();
+
+        try {
+            TransactionalMap<ResourcePath, List<ResourcePath>> childTxMap =
+                    tx.getTransactionalMap(CHILD_MAP, SERIALIZER);
+
+            if (!isRegistered(childTxMap, resource)) {
+                return abortTransaction(tx);
+            }
+
+            if (!appendValue(childTxMap, resource, children)) {
+                return abortTransaction(tx);
+            }
+
+            return commitTransaction(tx);
+        } catch (TransactionException e) {
+            log.error("Exception thrown, abort the transaction", e);
+            return abortTransaction(tx);
+        }
+    }
+
+    @Override
     public boolean allocate(List<ResourcePath> resources, ResourceConsumer consumer) {
     public boolean allocate(List<ResourcePath> resources, ResourceConsumer consumer) {
         checkNotNull(resources);
         checkNotNull(resources);
         checkNotNull(consumer);
         checkNotNull(consumer);
@@ -91,11 +129,18 @@ public class ConsistentResourceStore implements ResourceStore {
         tx.begin();
         tx.begin();
 
 
         try {
         try {
-            TransactionalMap<ResourcePath, ResourceConsumer> txMap = tx.getTransactionalMap(MAP_NAME, SERIALIZER);
+            TransactionalMap<ResourcePath, List<ResourcePath>> childTxMap =
+                    tx.getTransactionalMap(CHILD_MAP, SERIALIZER);
+            TransactionalMap<ResourcePath, ResourceConsumer> consumerTxMap =
+                    tx.getTransactionalMap(CONSUMER_MAP, SERIALIZER);
+
             for (ResourcePath resource: resources) {
             for (ResourcePath resource: resources) {
-                ResourceConsumer existing = txMap.putIfAbsent(resource, consumer);
-                // if the resource is already allocated to another consumer, the whole allocation fails
-                if (existing != null) {
+                if (!isRegistered(childTxMap, resource)) {
+                    return abortTransaction(tx);
+                }
+
+                ResourceConsumer oldValue = consumerTxMap.put(resource, consumer);
+                if (oldValue != null) {
                     return abortTransaction(tx);
                     return abortTransaction(tx);
                 }
                 }
             }
             }
@@ -117,7 +162,8 @@ public class ConsistentResourceStore implements ResourceStore {
         tx.begin();
         tx.begin();
 
 
         try {
         try {
-            TransactionalMap<ResourcePath, ResourceConsumer> txMap = tx.getTransactionalMap(MAP_NAME, SERIALIZER);
+            TransactionalMap<ResourcePath, ResourceConsumer> consumerTxMap =
+                    tx.getTransactionalMap(CONSUMER_MAP, SERIALIZER);
             Iterator<ResourcePath> resourceIte = resources.iterator();
             Iterator<ResourcePath> resourceIte = resources.iterator();
             Iterator<ResourceConsumer> consumerIte = consumers.iterator();
             Iterator<ResourceConsumer> consumerIte = consumers.iterator();
 
 
@@ -127,7 +173,7 @@ public class ConsistentResourceStore implements ResourceStore {
 
 
                 // if this single release fails (because the resource is allocated to another consumer,
                 // if this single release fails (because the resource is allocated to another consumer,
                 // the whole release fails
                 // the whole release fails
-                if (!txMap.remove(resource, consumer)) {
+                if (!consumerTxMap.remove(resource, consumer)) {
                     return abortTransaction(tx);
                     return abortTransaction(tx);
                 }
                 }
             }
             }
@@ -145,7 +191,7 @@ public class ConsistentResourceStore implements ResourceStore {
 
 
         // NOTE: getting all entries may become performance bottleneck
         // NOTE: getting all entries may become performance bottleneck
         // TODO: revisit for better backend data structure
         // TODO: revisit for better backend data structure
-        return consumers.entrySet().stream()
+        return consumerMap.entrySet().stream()
                 .filter(x -> x.getValue().value().equals(consumer))
                 .filter(x -> x.getValue().value().equals(consumer))
                 .map(Map.Entry::getKey)
                 .map(Map.Entry::getKey)
                 .collect(Collectors.toList());
                 .collect(Collectors.toList());
@@ -156,12 +202,14 @@ public class ConsistentResourceStore implements ResourceStore {
         checkNotNull(parent);
         checkNotNull(parent);
         checkNotNull(cls);
         checkNotNull(cls);
 
 
-        // NOTE: getting all entries may become performance bottleneck
-        // TODO: revisit for better backend data structure
-        return consumers.entrySet().stream()
-                .filter(x -> x.getKey().parent().isPresent() && x.getKey().parent().get().equals(parent))
-                .filter(x -> x.getKey().lastComponent().getClass() == cls)
-                .map(Map.Entry::getKey)
+        Versioned<List<ResourcePath>> children = childMap.get(parent);
+        if (children == null) {
+            return Collections.emptyList();
+        }
+
+        return children.value().stream()
+                .filter(x -> x.lastComponent().getClass().equals(cls))
+                .filter(consumerMap::containsKey)
                 .collect(Collectors.toList());
                 .collect(Collectors.toList());
     }
     }
 
 
@@ -186,4 +234,45 @@ public class ConsistentResourceStore implements ResourceStore {
         tx.commit();
         tx.commit();
         return true;
         return true;
     }
     }
+
+    /**
+     * Appends the values to the existing values associated with the specified key.
+     *
+     * @param map map holding multiple values for a key
+     * @param key key specifying values
+     * @param values values to be appended
+     * @param <K> type of the key
+     * @param <V> type of the element of the list
+     * @return true if the operation succeeds, false otherwise.
+     */
+    private <K, V> boolean appendValue(TransactionalMap<K, List<V>> map, K key, List<V> values) {
+        List<V> oldValues = map.get(key);
+        List<V> newValues;
+        if (oldValues == null) {
+            newValues = new ArrayList<>(values);
+        } else {
+            LinkedHashSet<V> newSet = new LinkedHashSet<>(oldValues);
+            newSet.addAll(values);
+            newValues = new ArrayList<>(newSet);
+        }
+
+        return map.replace(key, oldValues, newValues);
+    }
+
+    /**
+     * Checks if the specified resource is registered as a child of a resource in the map.
+     *
+     * @param map map storing parent - child relationship of resources
+     * @param resource resource to be checked
+     * @return true if the resource is registered, false otherwise.
+     */
+    private boolean isRegistered(TransactionalMap<ResourcePath, List<ResourcePath>> map, ResourcePath resource) {
+        // root is always regarded to be registered
+        if (!resource.parent().isPresent()) {
+            return true;
+        }
+
+        List<ResourcePath> value = map.get(resource.parent().get());
+        return value != null && value.contains(resource);
+    }
 }
 }