Introduce two specific types of ResourceId for Discrete and Continuous

Change-Id: I4a29beaabe32ba78fb03336192095edadc63e3c9

@@ -53,7 +53,7 @@ public abstract class Resource {
     public static final Discrete ROOT = new Discrete();
 
     public static Resource discrete(DeviceId device) {
-        return new Discrete(ResourceId.of(device));
+        return new Discrete(ResourceId.discrete(device));
     }
 
     /**
@@ -64,7 +64,7 @@ public abstract class Resource {
      * @return resource path instance
      */
     public static Resource discrete(DeviceId device, Object... components) {
-        return new Discrete(ResourceId.of(device, components));
+        return new Discrete(ResourceId.discrete(device, components));
     }
 
     /**
@@ -76,7 +76,7 @@ public abstract class Resource {
      * @return resource path instance
      */
     public static Resource discrete(DeviceId device, PortNumber port, Object... components) {
-        return new Discrete(ResourceId.of(device, port, components));
+        return new Discrete(ResourceId.discrete(device, port, components));
     }
 
     /**
@@ -85,13 +85,15 @@ public abstract class Resource {
      * @param value amount of the resource
      * @param device device ID which is the first component of the path
      * @param components following components of the path. The order represents hierarchical structure of the resource.
+     *                   The last element of this list must be an {@link Class} instance. Otherwise, this method throws
+     *                   an IllegalArgumentException.
      * @return resource path instance
      */
     public static Resource continuous(double value, DeviceId device, Object... components) {
         checkArgument(components.length > 0,
                 "Length of components must be greater thant 0, but " + components.length);
 
-        return new Continuous(ResourceId.of(device, components), value);
+        return new Continuous(ResourceId.continuous(device, components), value);
     }
 
     /**
@@ -101,10 +103,12 @@ public abstract class Resource {
      * @param device device ID which is the first component of the path.
      * @param port port number which is the second component of the path.
      * @param components following components of the path. The order represents hierarchical structure of the resource.
+     *                   The last element of this list must be an {@link Class} instance. Otherwise, this method throws
+     *                   an IllegalArgumentException.
      * @return resource path instance
      */
     public static Resource continuous(double value, DeviceId device, PortNumber port, Object... components) {
-        return new Continuous(ResourceId.of(device, port, components), value);
+        return new Continuous(ResourceId.continuous(device, port, components), value);
     }
 
     /**
@@ -139,6 +143,14 @@ public abstract class Resource {
     }
 
     /**
+     * Returns the volume of this resource.
+     *
+     * @return the volume of this resource
+     */
+    // TODO: think about other naming possibilities. amount? quantity?
+    public abstract <T> T volume();
+
+    /**
      * Returns the parent resource path of this instance.
      * E.g. if this path is Link:1/VLAN ID:100, the return value is the resource path for Link:1.
      *
@@ -199,30 +211,10 @@ public abstract class Resource {
     }
 
     @Override
-    public int hashCode() {
-        return id.hashCode();
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-        if (this == obj) {
-            return true;
-        }
-
-        if (obj == null) {
-            return false;
-        }
-        if (this.getClass() != obj.getClass()) {
-            return false;
-        }
-        final Resource that = (Resource) obj;
-        return Objects.equals(this.id, that.id);
-    }
-
-    @Override
     public String toString() {
         return MoreObjects.toStringHelper(this)
-                .add("id", id)
+                .add("id", id())
+                .add("volume", volume())
                 .toString();
     }
 
@@ -243,6 +235,39 @@ public abstract class Resource {
         private Discrete(ResourceId id) {
             super(id);
         }
+
+        /**
+         * The user of this methods must receive the return value as the correct type.
+         * Otherwise, this methods throws an exception.
+         *
+         * @param <T> type of the return value
+         * @return the volume of this resource
+         */
+        @SuppressWarnings("unchecked")
+        @Override
+        // TODO: consider receiving Class<T> as an argument. Which approach is convenient?
+        public <T> T volume() {
+            return (T) last();
+        }
+
+        @Override
+        public int hashCode() {
+            // the value returing from volume() is excluded due to optimization
+            return id().hashCode();
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            if (obj == null || getClass() != obj.getClass()) {
+                return false;
+            }
+            final Discrete other = (Discrete) obj;
+            // the value returing from volume() is excluded due to optimization
+            return Objects.equals(this.id(), other.id());
+        }
     }
 
     /**
@@ -261,16 +286,35 @@ public abstract class Resource {
             this.value = value;
         }
 
+        /**
+         * The user of this methods must receive the return value as Double or double.
+         * Otherwise, this methods throws an exception.
+         *
+         * @param <T> type of the return value
+         * @return the volume of this resource
+         */
+        @SuppressWarnings("unchecked")
+        @Override
+        public <T> T volume() {
+            return (T) Double.valueOf(value);
+        }
+
         @Override
         public int hashCode() {
-            return super.hashCode();
+            return Objects.hash(id(), value);
         }
 
-        // explicitly overriding to express that we intentionally ignore
-        // `value` in equality comparison
         @Override
         public boolean equals(Object obj) {
-            return super.equals(obj);
+            if (this == obj) {
+                return true;
+            }
+            if (obj == null || getClass() != obj.getClass()) {
+                return false;
+            }
+            final Continuous other = (Continuous) obj;
+            return Objects.equals(this.id(), other.id())
+                    && Objects.equals(this.value, other.value);
         }
 
         /**
@@ -278,17 +322,10 @@ public abstract class Resource {
          *
          * @return the value of the resource amount
          */
+        // FIXME: overlapping a purpose with volume()
         public double value() {
             return value;
         }
-
-        @Override
-        public String toString() {
-            return MoreObjects.toStringHelper(this)
-                    .add("id", id())
-                    .add("value", value)
-                    .toString();
-        }
     }
 
 }

@@ -20,35 +20,63 @@ import com.google.common.collect.ImmutableList;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.PortNumber;
 
+import java.util.Arrays;
 import java.util.Objects;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 
 /**
  * Represents identifier of resource.
  * This class is exposed to public, but intended to use only in ResourceStore implementations.
  */
 @Beta
-public final class ResourceId {
-    static final ResourceId ROOT = new ResourceId();
+public abstract class ResourceId {
+    static final ResourceId ROOT = new Discrete();
 
     final ImmutableList<Object> components;
 
-    static ResourceId of(DeviceId device, Object... components) {
-        return new ResourceId(ImmutableList.builder()
+    static ResourceId discrete(DeviceId device, Object... components) {
+        return new Discrete(ImmutableList.builder()
                 .add(device)
                 .add(components)
                 .build());
     }
 
-    static ResourceId of(DeviceId device, PortNumber port, Object... components) {
-        return new ResourceId(ImmutableList.builder()
+    static ResourceId discrete(DeviceId device, PortNumber port, Object... components) {
+        return new Discrete(ImmutableList.builder()
                 .add(device)
                 .add(port)
                 .add(components)
                 .build());
     }
 
+    static ResourceId continuous(DeviceId device, Object... components) {
+        Object last = components[components.length - 1];
+        checkArgument(last instanceof Class<?>);
+
+        return continuous(ImmutableList.builder()
+                .add(device)
+                .add(Arrays.copyOfRange(components, 0, components.length - 1)), (Class<?>) last);
+    }
+
+    static ResourceId continuous(DeviceId device, PortNumber port, Object... components) {
+        Object last = components[components.length - 1];
+        checkArgument(last instanceof Class<?>);
+
+        return continuous(ImmutableList.builder()
+                .add(device)
+                .add(port)
+                .add(Arrays.copyOfRange(components, 0, components.length - 1)), (Class<?>) last);
+    }
+
+    private static ResourceId continuous(ImmutableList.Builder<Object> parentComponents, Class<?> last) {
+        return new Continuous(parentComponents
+                .add(last.getCanonicalName())
+                .build(), last.getSimpleName());
+    }
+
     private ResourceId(ImmutableList<Object> components) {
         this.components = checkNotNull(components);
     }
@@ -63,15 +91,31 @@ public final class ResourceId {
         if (components.size() == 1) {
             return ROOT;
         } else {
-            return new ResourceId(components.subList(0, components.size() - 1));
+            return new Discrete(components.subList(0, components.size() - 1));
         }
     }
 
-    ResourceId child(Object child) {
-        return new ResourceId(ImmutableList.builder()
-                .addAll(components)
-                .add(child)
-                .build());
+    /**
+     * Returns a resource ID of a child of this resource based on the specified object.
+     * If the argument is an instance of {@link Class}, this method returns an instance of
+     * {@link Continuous}. Otherwise, it returns an instance of {@link Discrete}
+     * This method only work when the receiver is {@link Discrete}. Otherwise,
+     * this method throws an exception.
+     *
+     * @param child the last component of the child
+     * @return a child resource ID
+     */
+    public ResourceId child(Object child) {
+        checkState(this instanceof Discrete);
+
+        if (child instanceof Class<?>) {
+            return continuous(ImmutableList.builder().addAll(components), (Class<?>) child);
+        } else {
+            return new Discrete(ImmutableList.builder()
+                    .addAll(components)
+                    .add(child)
+                    .build());
+        }
     }
 
     @Override
@@ -84,11 +128,10 @@ public final class ResourceId {
         if (this == obj) {
             return true;
         }
-        if (!(obj instanceof ResourceId)) {
+        if (obj == null || getClass() != obj.getClass()) {
             return false;
         }
-
-        ResourceId other = (ResourceId) obj;
+        final ResourceId other = (ResourceId) obj;
         return Objects.equals(this.components, other.components);
     }
 
@@ -96,4 +139,45 @@ public final class ResourceId {
     public String toString() {
         return components.toString();
     }
+
+    /**
+     * ResourceId for {@link Resource.Discrete}.
+     *
+     * Note: This class is exposed to the public, but intended to be used in the resource API
+     * implementation only. It is not for resource API user.
+     */
+    public static final class Discrete extends ResourceId {
+        private Discrete(ImmutableList<Object> components) {
+            super(components);
+        }
+
+        private Discrete() {
+            super();
+        }
+    }
+
+    /**
+     * ResourceId for {@link Resource.Continuous}
+     *
+     * Note: This class is exposed to the public, but intended to be used in the resource API
+     * implementation only. It is not for resource API user.
+     */
+    public static final class Continuous extends ResourceId {
+        // for printing purpose only (used in toString() implementation)
+        private final String name;
+
+        private Continuous(ImmutableList<Object> components, String name) {
+            super(components);
+            this.name = checkNotNull(name);
+        }
+
+        @Override
+        public String toString() {
+            // due to performance consideration, the value might need to be stored in a field
+            return ImmutableList.builder()
+                    .addAll(components.subList(0, components.size() - 1))
+                    .add(name)
+                    .build().toString();
+        }
+    }
 }

@@ -132,6 +132,7 @@ public interface ResourceService extends ListenerService<ResourceEvent, Resource
      * @return list of allocation information.
      * If the resource is not allocated, the return value is an empty list.
      */
+    // TODO: need to change the argument type to ResourceId
     List<ResourceAllocation> getResourceAllocation(Resource resource);
 
     /**
@@ -143,6 +144,7 @@ public interface ResourceService extends ListenerService<ResourceEvent, Resource
      * @return non-empty collection of resource allocations if resources are allocated with the subject and type,
      * empty collection if no resource is allocated with the subject and type
      */
+    // TODO: might need to change the first argument type to ResourceId or ResourceId.Discrete
     <T> Collection<ResourceAllocation> getResourceAllocations(Resource parent, Class<T> cls);
 
     /**
@@ -159,6 +161,7 @@ public interface ResourceService extends ListenerService<ResourceEvent, Resource
      * @param parent parent resource
      * @return available resources under the specified resource
      */
+    // TODO: need to change the argument type to ResourceId or ResourceId.Discrete
     Collection<Resource> getAvailableResources(Resource parent);
 
     /**
@@ -167,6 +170,7 @@ public interface ResourceService extends ListenerService<ResourceEvent, Resource
      * @param parent parent resource
      * @return registered resources under the specified resource
      */
+    // TODO: need to change the argument type to ResourceId or ResourceId.Discrete
     Collection<Resource> getRegisteredResources(Resource parent);
 
 

@@ -84,6 +84,7 @@ public interface ResourceStore extends Store<ResourceEvent, ResourceStoreDelegat
      * @return resource consumers who are allocated the resource.
      * Returns empty list if there is no such consumer.
      */
+    // TODO: need to change the argument type to ResourceId
     List<ResourceConsumer> getConsumers(Resource resource);
 
     /**
@@ -108,6 +109,7 @@ public interface ResourceStore extends Store<ResourceEvent, ResourceStoreDelegat
      * @param parent parent of the resource to be returned
      * @return a collection of the child resources of the specified resource
      */
+    // TODO: need to change the argument type to ResourceId or ResourceId.Discrete
     Collection<Resource> getChildResources(Resource parent);
 
     /**
@@ -120,5 +122,6 @@ public interface ResourceStore extends Store<ResourceEvent, ResourceStoreDelegat
      * @return a collection of the resources which belongs to the specified subject and
      * whose type is the specified class.
      */
+    // TODO: need to change the argument type to ResourceId or ResourceId.Discrete
     <T> Collection<Resource> getAllocatedResources(Resource parent, Class<T> cls);
 }

+/*
+ * Copyright 2016 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;
+
+import org.junit.Test;
+import org.onlab.util.Bandwidth;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.PortNumber;
+
+import java.util.Arrays;
+
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+
+public class ResourceIdTest {
+    private static final DeviceId D1 = DeviceId.deviceId("a");
+    private static final PortNumber P1 = PortNumber.portNumber(1);
+    private static final Bandwidth BW1 = Bandwidth.gbps(1);
+
+    @Test
+    public void testDiscreteToString() {
+        ResourceId resource = ResourceId.discrete(D1, P1);
+
+        assertThat(resource.toString(), is(Arrays.asList(D1, P1).toString()));
+    }
+
+    @Test
+    public void testContinuousToString() {
+        ResourceId resource = ResourceId.continuous(D1, P1, Bandwidth.class);
+
+        assertThat(resource.toString(), is(Arrays.asList(D1, P1, Bandwidth.class.getSimpleName()).toString()));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInitWithNonClassInstance() {
+        ResourceId.continuous(D1, P1, BW1);
+    }
+}

@@ -42,8 +42,8 @@ public class ResourceTest {
         Resource resource1 = Resource.discrete(D1, P1, VLAN1);
         Resource sameAsResource1 = Resource.discrete(D1, P1, VLAN1);
         Resource resource2 = Resource.discrete(D2, P1, VLAN1);
-        Resource resource3 = Resource.continuous(BW1.bps(), D1, P1, BW1);
-        Resource sameAsResource3 = Resource.continuous(BW1.bps(), D1, P1, BW1);
+        Resource resource3 = Resource.continuous(BW1.bps(), D1, P1, Bandwidth.class);
+        Resource sameAsResource3 = Resource.continuous(BW1.bps(), D1, P1, Bandwidth.class);
 
         new EqualsTester()
                 .addEqualityGroup(resource1, sameAsResource1)
@@ -64,9 +64,9 @@ public class ResourceTest {
         ResourceId id1 = Resource.discrete(D1, P1, VLAN1).id();
         ResourceId sameAsId1 = Resource.discrete(D1, P1, VLAN1).id();
         ResourceId id2 = Resource.discrete(D2, P1, VLAN1).id();
-        ResourceId id3 = Resource.continuous(BW1.bps(), D1, P1, BW1).id();
+        ResourceId id3 = Resource.continuous(BW1.bps(), D1, P1, Bandwidth.class).id();
         // intentionally set a different value
-        ResourceId sameAsId3 = Resource.continuous(BW2.bps(), D1, P1, BW1).id();
+        ResourceId sameAsId3 = Resource.continuous(BW2.bps(), D1, P1, Bandwidth.class).id();
 
         new EqualsTester()
                 .addEqualityGroup(id1, sameAsId1)
@@ -104,4 +104,20 @@ public class ResourceTest {
         DeviceId child = (DeviceId) resource.last();
         assertThat(child, is(D1));
     }
+
+    @Test
+    public void testVolumeOfDiscrete() {
+        Resource resource = Resource.discrete(D1);
+
+        DeviceId volume = resource.volume();
+        assertThat(volume, is(D1));
+    }
+
+    @Test
+    public void testVolumeOfContinuous() {
+        Resource resource = Resource.continuous(BW1.bps(), D1, P1, Bandwidth.class);
+
+        double volume = resource.volume();
+        assertThat(volume, is(BW1.bps()));
+    }
 }

@@ -339,7 +339,7 @@ public class ConsistentResourceStore extends AbstractStore<ResourceEvent, Resour
         } else {
             Resource.Continuous requested = (Resource.Continuous) resource;
             Resource.Continuous registered = v.value().stream()
-                    .filter(c -> c.equals(resource))
+                    .filter(c -> c.id().equals(resource.id()))
                     .findFirst()
                     .map(c -> (Resource.Continuous) c)
                     .get();
@@ -415,7 +415,7 @@ public class ConsistentResourceStore extends AbstractStore<ResourceEvent, Resour
                 .filter(discreteConsumers::containsKey);
 
         Stream<Resource.Continuous> continuous = children.value().stream()
-                .filter(x -> x.last().getClass().equals(cls))
+                .filter(x -> x.id().equals(parent.id().child(cls)))
                 .filter(x -> x instanceof Resource.Continuous)
                 .map(x -> (Resource.Continuous) x)
                 .filter(x -> continuousConsumers.containsKey(x.id()))

@@ -437,6 +437,8 @@ public final class KryoNamespaces {
                     Resource.Discrete.class,
                     Resource.Continuous.class,
                     ResourceId.class,
+                    ResourceId.Discrete.class,
+                    ResourceId.Continuous.class,
                     ResourceAllocation.class,
                     // Constraints
                     LambdaConstraint.class,