Adding more unit tests.
Made some classes abstract which should have been.
Showing
10 changed files
with
169 additions
and
19 deletions
| ... | @@ -5,7 +5,7 @@ import static com.google.common.base.Preconditions.checkArgument; | ... | @@ -5,7 +5,7 @@ import static com.google.common.base.Preconditions.checkArgument; |
| 5 | /** | 5 | /** |
| 6 | * Base abstraction of an annotated entity. | 6 | * Base abstraction of an annotated entity. |
| 7 | */ | 7 | */ |
| 8 | -public class AbstractAnnotated implements Annotated { | 8 | +public abstract class AbstractAnnotated implements Annotated { |
| 9 | 9 | ||
| 10 | private static final Annotations EMPTY = DefaultAnnotations.builder().build(); | 10 | private static final Annotations EMPTY = DefaultAnnotations.builder().build(); |
| 11 | 11 | ... | ... |
| ... | @@ -5,7 +5,7 @@ import static com.google.common.base.Preconditions.checkArgument; | ... | @@ -5,7 +5,7 @@ import static com.google.common.base.Preconditions.checkArgument; |
| 5 | /** | 5 | /** |
| 6 | * Base implementation of an annotated model description. | 6 | * Base implementation of an annotated model description. |
| 7 | */ | 7 | */ |
| 8 | -public class AbstractDescription implements Annotated { | 8 | +public abstract class AbstractDescription implements Annotated { |
| 9 | 9 | ||
| 10 | private static final SparseAnnotations EMPTY = DefaultAnnotations.builder().build(); | 10 | private static final SparseAnnotations EMPTY = DefaultAnnotations.builder().build(); |
| 11 | 11 | ... | ... |
| ... | @@ -5,7 +5,7 @@ import org.onlab.onos.net.provider.ProviderId; | ... | @@ -5,7 +5,7 @@ import org.onlab.onos.net.provider.ProviderId; |
| 5 | /** | 5 | /** |
| 6 | * Base implementation of network elements, i.e. devices or hosts. | 6 | * Base implementation of network elements, i.e. devices or hosts. |
| 7 | */ | 7 | */ |
| 8 | -public class AbstractElement extends AbstractModel implements Element { | 8 | +public abstract class AbstractElement extends AbstractModel implements Element { |
| 9 | 9 | ||
| 10 | protected final ElementId id; | 10 | protected final ElementId id; |
| 11 | 11 | ||
| ... | @@ -27,9 +27,4 @@ public class AbstractElement extends AbstractModel implements Element { | ... | @@ -27,9 +27,4 @@ public class AbstractElement extends AbstractModel implements Element { |
| 27 | this.id = id; | 27 | this.id = id; |
| 28 | } | 28 | } |
| 29 | 29 | ||
| 30 | - @Override | ||
| 31 | - public ElementId id() { | ||
| 32 | - return id; | ||
| 33 | - } | ||
| 34 | - | ||
| 35 | } | 30 | } | ... | ... |
| ... | @@ -5,7 +5,7 @@ import org.onlab.onos.net.provider.ProviderId; | ... | @@ -5,7 +5,7 @@ import org.onlab.onos.net.provider.ProviderId; |
| 5 | /** | 5 | /** |
| 6 | * Base implementation of a network model entity. | 6 | * Base implementation of a network model entity. |
| 7 | */ | 7 | */ |
| 8 | -public class AbstractModel extends AbstractAnnotated implements Provided { | 8 | +public abstract class AbstractModel extends AbstractAnnotated implements Provided { |
| 9 | 9 | ||
| 10 | private final ProviderId providerId; | 10 | private final ProviderId providerId; |
| 11 | 11 | ... | ... |
| ... | @@ -5,6 +5,8 @@ import java.util.Map; | ... | @@ -5,6 +5,8 @@ import java.util.Map; |
| 5 | import java.util.Objects; | 5 | import java.util.Objects; |
| 6 | import java.util.Set; | 6 | import java.util.Set; |
| 7 | 7 | ||
| 8 | +import static com.google.common.base.Preconditions.checkNotNull; | ||
| 9 | + | ||
| 8 | /** | 10 | /** |
| 9 | * Represents a set of simple annotations that can be used to add arbitrary | 11 | * Represents a set of simple annotations that can be used to add arbitrary |
| 10 | * attributes to various parts of the data model. | 12 | * attributes to various parts of the data model. |
| ... | @@ -19,9 +21,9 @@ public final class DefaultAnnotations implements SparseAnnotations { | ... | @@ -19,9 +21,9 @@ public final class DefaultAnnotations implements SparseAnnotations { |
| 19 | } | 21 | } |
| 20 | 22 | ||
| 21 | /** | 23 | /** |
| 22 | - * Creates a new set of annotations using the specified immutable map. | 24 | + * Creates a new set of annotations using clone of the specified hash map. |
| 23 | * | 25 | * |
| 24 | - * @param map immutable map of key/value pairs | 26 | + * @param map hash map of key/value pairs |
| 25 | */ | 27 | */ |
| 26 | private DefaultAnnotations(Map<String, String> map) { | 28 | private DefaultAnnotations(Map<String, String> map) { |
| 27 | this.map = map; | 29 | this.map = map; |
| ... | @@ -36,6 +38,38 @@ public final class DefaultAnnotations implements SparseAnnotations { | ... | @@ -36,6 +38,38 @@ public final class DefaultAnnotations implements SparseAnnotations { |
| 36 | return new Builder(); | 38 | return new Builder(); |
| 37 | } | 39 | } |
| 38 | 40 | ||
| 41 | + /** | ||
| 42 | + * Merges the specified base set of annotations and additional sparse | ||
| 43 | + * annotations into new combined annotations. If the supplied sparse | ||
| 44 | + * annotations are empty, the original base annotations are returned. | ||
| 45 | + * Any keys tagged for removal in the sparse annotations will be omitted | ||
| 46 | + * in the resulting merged annotations. | ||
| 47 | + * | ||
| 48 | + * @param annotations base annotations | ||
| 49 | + * @param sparseAnnotations additional sparse annotations | ||
| 50 | + * @return combined annotations or the original base annotations if there | ||
| 51 | + * are not additional annotations | ||
| 52 | + */ | ||
| 53 | + public static DefaultAnnotations merge(DefaultAnnotations annotations, | ||
| 54 | + SparseAnnotations sparseAnnotations) { | ||
| 55 | + checkNotNull(annotations, "Annotations cannot be null"); | ||
| 56 | + if (sparseAnnotations == null || sparseAnnotations.keys().isEmpty()) { | ||
| 57 | + return annotations; | ||
| 58 | + } | ||
| 59 | + | ||
| 60 | + // Merge the two maps. Yes, this is not very efficient, but the | ||
| 61 | + // use-case implies small maps and infrequent merges, so we opt for | ||
| 62 | + // simplicity. | ||
| 63 | + Map<String, String> merged = copy(annotations.map); | ||
| 64 | + for (String key : sparseAnnotations.keys()) { | ||
| 65 | + if (sparseAnnotations.isRemoved(key)) { | ||
| 66 | + merged.remove(key); | ||
| 67 | + } else { | ||
| 68 | + merged.put(key, sparseAnnotations.value(key)); | ||
| 69 | + } | ||
| 70 | + } | ||
| 71 | + return new DefaultAnnotations(merged); | ||
| 72 | + } | ||
| 39 | 73 | ||
| 40 | @Override | 74 | @Override |
| 41 | public Set<String> keys() { | 75 | public Set<String> keys() { |
| ... | @@ -53,15 +87,20 @@ public final class DefaultAnnotations implements SparseAnnotations { | ... | @@ -53,15 +87,20 @@ public final class DefaultAnnotations implements SparseAnnotations { |
| 53 | return Objects.equals(Builder.REMOVED, map.get(key)); | 87 | return Objects.equals(Builder.REMOVED, map.get(key)); |
| 54 | } | 88 | } |
| 55 | 89 | ||
| 90 | + @SuppressWarnings("unchecked") | ||
| 91 | + private static HashMap<String, String> copy(Map<String, String> original) { | ||
| 92 | + if (original instanceof HashMap) { | ||
| 93 | + return (HashMap) ((HashMap) original).clone(); | ||
| 94 | + } | ||
| 95 | + throw new IllegalArgumentException("Expecting HashMap instance"); | ||
| 96 | + } | ||
| 97 | + | ||
| 56 | /** | 98 | /** |
| 57 | * Facility for gradually building model annotations. | 99 | * Facility for gradually building model annotations. |
| 58 | */ | 100 | */ |
| 59 | public static final class Builder { | 101 | public static final class Builder { |
| 60 | 102 | ||
| 61 | private static final String REMOVED = "~rEmOvEd~"; | 103 | private static final String REMOVED = "~rEmOvEd~"; |
| 62 | - | ||
| 63 | - // FIXME: Figure out whether and how to make this immutable and serializable | ||
| 64 | -// private final ImmutableMap.Builder<String, String> builder = ImmutableMap.builder(); | ||
| 65 | private final Map<String, String> builder = new HashMap<>(); | 104 | private final Map<String, String> builder = new HashMap<>(); |
| 66 | 105 | ||
| 67 | // Private construction is forbidden. | 106 | // Private construction is forbidden. |
| ... | @@ -99,8 +138,7 @@ public final class DefaultAnnotations implements SparseAnnotations { | ... | @@ -99,8 +138,7 @@ public final class DefaultAnnotations implements SparseAnnotations { |
| 99 | * @return annotations | 138 | * @return annotations |
| 100 | */ | 139 | */ |
| 101 | public DefaultAnnotations build() { | 140 | public DefaultAnnotations build() { |
| 102 | -// return new DefaultAnnotations(builder.build()); | 141 | + return new DefaultAnnotations(copy(builder)); |
| 103 | - return new DefaultAnnotations(builder); | ||
| 104 | } | 142 | } |
| 105 | } | 143 | } |
| 106 | } | 144 | } | ... | ... |
| ... | @@ -51,7 +51,7 @@ public class DefaultDevice extends AbstractElement implements Device { | ... | @@ -51,7 +51,7 @@ public class DefaultDevice extends AbstractElement implements Device { |
| 51 | 51 | ||
| 52 | @Override | 52 | @Override |
| 53 | public DeviceId id() { | 53 | public DeviceId id() { |
| 54 | - return (DeviceId) super.id(); | 54 | + return (DeviceId) id; |
| 55 | } | 55 | } |
| 56 | 56 | ||
| 57 | @Override | 57 | @Override | ... | ... |
| ... | @@ -45,7 +45,7 @@ public class DefaultHost extends AbstractElement implements Host { | ... | @@ -45,7 +45,7 @@ public class DefaultHost extends AbstractElement implements Host { |
| 45 | 45 | ||
| 46 | @Override | 46 | @Override |
| 47 | public HostId id() { | 47 | public HostId id() { |
| 48 | - return (HostId) super.id(); | 48 | + return (HostId) id; |
| 49 | } | 49 | } |
| 50 | 50 | ||
| 51 | @Override | 51 | @Override | ... | ... |
| 1 | +package org.onlab.onos.net; | ||
| 2 | + | ||
| 3 | +import com.google.common.testing.EqualsTester; | ||
| 4 | +import org.junit.Test; | ||
| 5 | + | ||
| 6 | +import static org.junit.Assert.*; | ||
| 7 | +import static org.onlab.onos.net.Device.Type.SWITCH; | ||
| 8 | +import static org.onlab.onos.net.DeviceId.deviceId; | ||
| 9 | +import static org.onlab.onos.net.PortNumber.portNumber; | ||
| 10 | + | ||
| 11 | +/** | ||
| 12 | + * Test of the connetion point entity. | ||
| 13 | + */ | ||
| 14 | +public class ConnectPointTest { | ||
| 15 | + | ||
| 16 | + public static final DeviceId DID1 = deviceId("1"); | ||
| 17 | + public static final DeviceId DID2 = deviceId("2"); | ||
| 18 | + public static final PortNumber P1 = portNumber(1); | ||
| 19 | + public static final PortNumber P2 = portNumber(2); | ||
| 20 | + | ||
| 21 | + @Test | ||
| 22 | + public void basics() { | ||
| 23 | + ConnectPoint p = new ConnectPoint(DID1, P2); | ||
| 24 | + assertEquals("incorrect element id", DID1, p.deviceId()); | ||
| 25 | + assertEquals("incorrect element id", P2, p.port()); | ||
| 26 | + } | ||
| 27 | + | ||
| 28 | + | ||
| 29 | + @Test | ||
| 30 | + public void testEquality() { | ||
| 31 | + new EqualsTester() | ||
| 32 | + .addEqualityGroup(new ConnectPoint(DID1, P1), new ConnectPoint(DID1, P1)) | ||
| 33 | + .addEqualityGroup(new ConnectPoint(DID1, P2), new ConnectPoint(DID1, P2)) | ||
| 34 | + .addEqualityGroup(new ConnectPoint(DID2, P1), new ConnectPoint(DID2, P1)) | ||
| 35 | + .testEquals(); | ||
| 36 | + } | ||
| 37 | +} | ||
| ... | \ No newline at end of file | ... | \ No newline at end of file |
| 1 | +package org.onlab.onos.net; | ||
| 2 | + | ||
| 3 | +import org.junit.Test; | ||
| 4 | + | ||
| 5 | +import static com.google.common.collect.ImmutableSet.of; | ||
| 6 | +import static org.junit.Assert.*; | ||
| 7 | +import static org.onlab.onos.net.DefaultAnnotations.builder; | ||
| 8 | + | ||
| 9 | +/** | ||
| 10 | + * Tests of the default annotations. | ||
| 11 | + */ | ||
| 12 | +public class DefaultAnnotationsTest { | ||
| 13 | + | ||
| 14 | + private DefaultAnnotations annotations; | ||
| 15 | + | ||
| 16 | + @Test | ||
| 17 | + public void basics() { | ||
| 18 | + annotations = builder().set("foo", "1").set("bar", "2").build(); | ||
| 19 | + assertEquals("incorrect keys", of("foo", "bar"), annotations.keys()); | ||
| 20 | + assertEquals("incorrect value", "1", annotations.value("foo")); | ||
| 21 | + assertEquals("incorrect value", "2", annotations.value("bar")); | ||
| 22 | + } | ||
| 23 | + | ||
| 24 | + @Test | ||
| 25 | + public void empty() { | ||
| 26 | + annotations = builder().build(); | ||
| 27 | + assertTrue("incorrect keys", annotations.keys().isEmpty()); | ||
| 28 | + } | ||
| 29 | + | ||
| 30 | + @Test | ||
| 31 | + public void remove() { | ||
| 32 | + annotations = builder().remove("foo").set("bar", "2").build(); | ||
| 33 | + assertEquals("incorrect keys", of("foo", "bar"), annotations.keys()); | ||
| 34 | + assertNull("incorrect value", annotations.value("foo")); | ||
| 35 | + assertEquals("incorrect value", "2", annotations.value("bar")); | ||
| 36 | + } | ||
| 37 | + | ||
| 38 | + @Test | ||
| 39 | + public void merge() { | ||
| 40 | + annotations = builder().set("foo", "1").set("bar", "2").build(); | ||
| 41 | + assertEquals("incorrect keys", of("foo", "bar"), annotations.keys()); | ||
| 42 | + | ||
| 43 | + SparseAnnotations updates = builder().remove("foo").set("bar", "3").set("goo", "4").build(); | ||
| 44 | + | ||
| 45 | + annotations = DefaultAnnotations.merge(annotations, updates); | ||
| 46 | + assertEquals("incorrect keys", of("goo", "bar"), annotations.keys()); | ||
| 47 | + assertNull("incorrect value", annotations.value("foo")); | ||
| 48 | + assertEquals("incorrect value", "3", annotations.value("bar")); | ||
| 49 | + } | ||
| 50 | + | ||
| 51 | + @Test | ||
| 52 | + public void noopMerge() { | ||
| 53 | + annotations = builder().set("foo", "1").set("bar", "2").build(); | ||
| 54 | + assertEquals("incorrect keys", of("foo", "bar"), annotations.keys()); | ||
| 55 | + | ||
| 56 | + SparseAnnotations updates = builder().build(); | ||
| 57 | + assertSame("same annotations expected", annotations, | ||
| 58 | + DefaultAnnotations.merge(annotations, updates)); | ||
| 59 | + assertSame("same annotations expected", annotations, | ||
| 60 | + DefaultAnnotations.merge(annotations, null)); | ||
| 61 | + } | ||
| 62 | + | ||
| 63 | + @Test(expected = NullPointerException.class) | ||
| 64 | + public void badMerge() { | ||
| 65 | + DefaultAnnotations.merge(null, null); | ||
| 66 | + } | ||
| 67 | + | ||
| 68 | +} | ||
| ... | \ No newline at end of file | ... | \ No newline at end of file |
| ... | @@ -40,6 +40,18 @@ public class DefaultDeviceTest { | ... | @@ -40,6 +40,18 @@ public class DefaultDeviceTest { |
| 40 | @Test | 40 | @Test |
| 41 | public void basics() { | 41 | public void basics() { |
| 42 | Device device = new DefaultDevice(PID, DID1, SWITCH, MFR, HW, SW, SN1); | 42 | Device device = new DefaultDevice(PID, DID1, SWITCH, MFR, HW, SW, SN1); |
| 43 | + validate(device); | ||
| 44 | + } | ||
| 45 | + | ||
| 46 | + @Test | ||
| 47 | + public void annotations() { | ||
| 48 | + Device device = new DefaultDevice(PID, DID1, SWITCH, MFR, HW, SW, SN1, | ||
| 49 | + DefaultAnnotations.builder().set("foo", "bar").build()); | ||
| 50 | + validate(device); | ||
| 51 | + assertEquals("incorrect provider", "bar", device.annotations().value("foo")); | ||
| 52 | + } | ||
| 53 | + | ||
| 54 | + private void validate(Device device) { | ||
| 43 | assertEquals("incorrect provider", PID, device.providerId()); | 55 | assertEquals("incorrect provider", PID, device.providerId()); |
| 44 | assertEquals("incorrect id", DID1, device.id()); | 56 | assertEquals("incorrect id", DID1, device.id()); |
| 45 | assertEquals("incorrect type", SWITCH, device.type()); | 57 | assertEquals("incorrect type", SWITCH, device.type()); |
| ... | @@ -50,4 +62,4 @@ public class DefaultDeviceTest { | ... | @@ -50,4 +62,4 @@ public class DefaultDeviceTest { |
| 50 | assertEquals("incorrect serial", SN1, device.serialNumber()); | 62 | assertEquals("incorrect serial", SN1, device.serialNumber()); |
| 51 | } | 63 | } |
| 52 | 64 | ||
| 53 | -} | 65 | +} |
| ... | \ No newline at end of file | ... | \ No newline at end of file | ... | ... |
-
Please register or login to post a comment