Committed by
Gerrit Code Review
Fix for ONOS-5175. GroupId refactoring.
Change-Id: I951392bdc69fe1ef694d321164b0b49032617d6b
Showing
7 changed files
with
83 additions
and
29 deletions
| ... | @@ -22,28 +22,21 @@ import java.util.Objects; | ... | @@ -22,28 +22,21 @@ import java.util.Objects; |
| 22 | /** | 22 | /** |
| 23 | * Default implementation of {@link GroupId}. | 23 | * Default implementation of {@link GroupId}. |
| 24 | */ | 24 | */ |
| 25 | -// TODO: require refactor to extend from Identifier base class | 25 | +@Deprecated |
| 26 | -public class DefaultGroupId implements GroupId { | 26 | +public class DefaultGroupId extends GroupId { |
| 27 | - | ||
| 28 | - private final int id; | ||
| 29 | 27 | ||
| 30 | public DefaultGroupId(int id) { | 28 | public DefaultGroupId(int id) { |
| 31 | - this.id = id; | 29 | + super(id); |
| 32 | } | 30 | } |
| 33 | 31 | ||
| 34 | // Constructor for serialization | 32 | // Constructor for serialization |
| 35 | private DefaultGroupId() { | 33 | private DefaultGroupId() { |
| 36 | - this.id = 0; | 34 | + super(0); |
| 37 | - } | ||
| 38 | - | ||
| 39 | - @Override | ||
| 40 | - public int id() { | ||
| 41 | - return this.id; | ||
| 42 | } | 35 | } |
| 43 | 36 | ||
| 44 | @Override | 37 | @Override |
| 45 | public int hashCode() { | 38 | public int hashCode() { |
| 46 | - return id; | 39 | + return identifier; |
| 47 | } | 40 | } |
| 48 | 41 | ||
| 49 | @Override | 42 | @Override |
| ... | @@ -55,13 +48,13 @@ public class DefaultGroupId implements GroupId { | ... | @@ -55,13 +48,13 @@ public class DefaultGroupId implements GroupId { |
| 55 | return false; | 48 | return false; |
| 56 | } | 49 | } |
| 57 | final DefaultGroupId other = (DefaultGroupId) obj; | 50 | final DefaultGroupId other = (DefaultGroupId) obj; |
| 58 | - return Objects.equals(this.id, other.id); | 51 | + return Objects.equals(this.identifier, other.identifier); |
| 59 | } | 52 | } |
| 60 | 53 | ||
| 61 | @Override | 54 | @Override |
| 62 | public String toString() { | 55 | public String toString() { |
| 63 | return MoreObjects.toStringHelper(this) | 56 | return MoreObjects.toStringHelper(this) |
| 64 | - .add("id", "0x" + Integer.toHexString(id)) | 57 | + .add("id", "0x" + Integer.toHexString(identifier)) |
| 65 | .toString(); | 58 | .toString(); |
| 66 | } | 59 | } |
| 67 | } | 60 | } | ... | ... |
| ... | @@ -15,18 +15,39 @@ | ... | @@ -15,18 +15,39 @@ |
| 15 | */ | 15 | */ |
| 16 | package org.onosproject.core; | 16 | package org.onosproject.core; |
| 17 | 17 | ||
| 18 | +import com.google.common.base.MoreObjects; | ||
| 19 | +import org.onlab.util.Identifier; | ||
| 20 | + | ||
| 18 | /** | 21 | /** |
| 19 | * Group identifier. | 22 | * Group identifier. |
| 20 | */ | 23 | */ |
| 21 | -// TODO: require refactor to extend from Identifier base class | 24 | +public class GroupId extends Identifier<Integer> { |
| 22 | -public interface GroupId { | 25 | + |
| 26 | + public GroupId(int id) { | ||
| 27 | + super(id); | ||
| 28 | + } | ||
| 29 | + | ||
| 30 | + // Constructor for serialization | ||
| 31 | + private GroupId() { | ||
| 32 | + super(0); | ||
| 33 | + } | ||
| 23 | 34 | ||
| 24 | /** | 35 | /** |
| 25 | * Returns a group ID as an integer value. | 36 | * Returns a group ID as an integer value. |
| 26 | * The method is not intended for use by application developers. | 37 | * The method is not intended for use by application developers. |
| 27 | * Return data type may change in the future release. | 38 | * Return data type may change in the future release. |
| 28 | * | 39 | * |
| 29 | - * @return a group ID as integer value | 40 | + * @param id int value |
| 41 | + * @return group ID | ||
| 30 | */ | 42 | */ |
| 31 | - int id(); | 43 | + public static GroupId valueOf(int id) { |
| 44 | + return new GroupId(id); | ||
| 45 | + } | ||
| 46 | + | ||
| 47 | + @Override | ||
| 48 | + public String toString() { | ||
| 49 | + return MoreObjects.toStringHelper(this) | ||
| 50 | + .add("id", "0x" + Integer.toHexString(identifier)) | ||
| 51 | + .toString(); | ||
| 52 | + } | ||
| 32 | } | 53 | } | ... | ... |
| 1 | +/* | ||
| 2 | + * Copyright 2016 Open Networking Laboratory | ||
| 3 | + * | ||
| 4 | + * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| 5 | + * you may not use this file except in compliance with the License. | ||
| 6 | + * You may obtain a copy of the License at | ||
| 7 | + * | ||
| 8 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
| 9 | + * | ||
| 10 | + * Unless required by applicable law or agreed to in writing, software | ||
| 11 | + * distributed under the License is distributed on an "AS IS" BASIS, | ||
| 12 | + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| 13 | + * See the License for the specific language governing permissions and | ||
| 14 | + * limitations under the License. | ||
| 15 | + */ | ||
| 16 | +package org.onosproject.core; | ||
| 17 | + | ||
| 18 | +import com.google.common.testing.EqualsTester; | ||
| 19 | +import org.junit.Test; | ||
| 20 | + | ||
| 21 | +/** | ||
| 22 | + * Test for GroupId. | ||
| 23 | + */ | ||
| 24 | +public class GroupIdTest { | ||
| 25 | + | ||
| 26 | + /** | ||
| 27 | + * Tests the equality of the instances. | ||
| 28 | + */ | ||
| 29 | + @Test | ||
| 30 | + public void testEquality() { | ||
| 31 | + GroupId id1 = new GroupId((short) 1); | ||
| 32 | + GroupId id2 = new GroupId((short) 1); | ||
| 33 | + GroupId id3 = new GroupId((short) 2); | ||
| 34 | + | ||
| 35 | + new EqualsTester() | ||
| 36 | + .addEqualityGroup(id1, id2) | ||
| 37 | + .addEqualityGroup(id3) | ||
| 38 | + .testEquals(); | ||
| 39 | + } | ||
| 40 | + | ||
| 41 | +} |
| ... | @@ -23,7 +23,6 @@ import org.onosproject.codec.CodecContext; | ... | @@ -23,7 +23,6 @@ import org.onosproject.codec.CodecContext; |
| 23 | import org.onosproject.codec.JsonCodec; | 23 | import org.onosproject.codec.JsonCodec; |
| 24 | import org.onosproject.core.ApplicationId; | 24 | import org.onosproject.core.ApplicationId; |
| 25 | import org.onosproject.core.CoreService; | 25 | import org.onosproject.core.CoreService; |
| 26 | -import org.onosproject.core.DefaultGroupId; | ||
| 27 | import org.onosproject.core.GroupId; | 26 | import org.onosproject.core.GroupId; |
| 28 | import org.onosproject.net.DeviceId; | 27 | import org.onosproject.net.DeviceId; |
| 29 | import org.onosproject.net.group.DefaultGroup; | 28 | import org.onosproject.net.group.DefaultGroup; |
| ... | @@ -72,7 +71,7 @@ public final class GroupCodec extends JsonCodec<Group> { | ... | @@ -72,7 +71,7 @@ public final class GroupCodec extends JsonCodec<Group> { |
| 72 | public ObjectNode encode(Group group, CodecContext context) { | 71 | public ObjectNode encode(Group group, CodecContext context) { |
| 73 | checkNotNull(group, "Group cannot be null"); | 72 | checkNotNull(group, "Group cannot be null"); |
| 74 | ObjectNode result = context.mapper().createObjectNode() | 73 | ObjectNode result = context.mapper().createObjectNode() |
| 75 | - .put(ID, group.id().toString()) | 74 | + .put(ID, group.id().id().toString()) |
| 76 | .put(STATE, group.state().toString()) | 75 | .put(STATE, group.state().toString()) |
| 77 | .put(LIFE, group.life()) | 76 | .put(LIFE, group.life()) |
| 78 | .put(PACKETS, group.packets()) | 77 | .put(PACKETS, group.packets()) |
| ... | @@ -90,7 +89,7 @@ public final class GroupCodec extends JsonCodec<Group> { | ... | @@ -90,7 +89,7 @@ public final class GroupCodec extends JsonCodec<Group> { |
| 90 | } | 89 | } |
| 91 | 90 | ||
| 92 | if (group.givenGroupId() != null) { | 91 | if (group.givenGroupId() != null) { |
| 93 | - result.put(GIVEN_GROUP_ID, group.givenGroupId()); | 92 | + result.put(GIVEN_GROUP_ID, group.givenGroupId().toString()); |
| 94 | } | 93 | } |
| 95 | 94 | ||
| 96 | ArrayNode buckets = context.mapper().createArrayNode(); | 95 | ArrayNode buckets = context.mapper().createArrayNode(); |
| ... | @@ -114,7 +113,7 @@ public final class GroupCodec extends JsonCodec<Group> { | ... | @@ -114,7 +113,7 @@ public final class GroupCodec extends JsonCodec<Group> { |
| 114 | // parse group id | 113 | // parse group id |
| 115 | int groupIdInt = nullIsIllegal(json.get(GROUP_ID), | 114 | int groupIdInt = nullIsIllegal(json.get(GROUP_ID), |
| 116 | GROUP_ID + MISSING_MEMBER_MESSAGE).asInt(); | 115 | GROUP_ID + MISSING_MEMBER_MESSAGE).asInt(); |
| 117 | - GroupId groupId = new DefaultGroupId(groupIdInt); | 116 | + GroupId groupId = new GroupId(groupIdInt); |
| 118 | 117 | ||
| 119 | // parse group key (appCookie) | 118 | // parse group key (appCookie) |
| 120 | String groupKeyStr = nullIsIllegal(json.get(APP_COOKIE), | 119 | String groupKeyStr = nullIsIllegal(json.get(APP_COOKIE), | ... | ... |
| ... | @@ -38,7 +38,7 @@ public final class GroupJsonMatcher extends TypeSafeDiagnosingMatcher<JsonNode> | ... | @@ -38,7 +38,7 @@ public final class GroupJsonMatcher extends TypeSafeDiagnosingMatcher<JsonNode> |
| 38 | public boolean matchesSafely(JsonNode jsonGroup, Description description) { | 38 | public boolean matchesSafely(JsonNode jsonGroup, Description description) { |
| 39 | // check id | 39 | // check id |
| 40 | String jsonGroupId = jsonGroup.get("id").asText(); | 40 | String jsonGroupId = jsonGroup.get("id").asText(); |
| 41 | - String groupId = group.id().toString(); | 41 | + String groupId = group.id().id().toString(); |
| 42 | if (!jsonGroupId.equals(groupId)) { | 42 | if (!jsonGroupId.equals(groupId)) { |
| 43 | description.appendText("group id was " + jsonGroupId); | 43 | description.appendText("group id was " + jsonGroupId); |
| 44 | return false; | 44 | return false; | ... | ... |
| ... | @@ -361,8 +361,8 @@ public class GroupManagerTest { | ... | @@ -361,8 +361,8 @@ public class GroupManagerTest { |
| 361 | GroupKey key = new DefaultGroupKey("group1BeforeAudit".getBytes()); | 361 | GroupKey key = new DefaultGroupKey("group1BeforeAudit".getBytes()); |
| 362 | Group createdGroup = groupService.getGroup(deviceId, key); | 362 | Group createdGroup = groupService.getGroup(deviceId, key); |
| 363 | int createdGroupId = createdGroup.id().id(); | 363 | int createdGroupId = createdGroup.id().id(); |
| 364 | - assertNotEquals(gId1.id(), createdGroupId); | 364 | + assertNotEquals(gId1.id().intValue(), createdGroupId); |
| 365 | - assertNotEquals(gId2.id(), createdGroupId); | 365 | + assertNotEquals(gId2.id().intValue(), createdGroupId); |
| 366 | 366 | ||
| 367 | List<GroupOperation> expectedGroupOps = Arrays.asList( | 367 | List<GroupOperation> expectedGroupOps = Arrays.asList( |
| 368 | GroupOperation.createDeleteGroupOperation(gId1, | 368 | GroupOperation.createDeleteGroupOperation(gId1, | ... | ... |
| ... | @@ -272,9 +272,9 @@ public class GroupsResourceTest extends ResourceTest { | ... | @@ -272,9 +272,9 @@ public class GroupsResourceTest extends ResourceTest { |
| 272 | public boolean matchesSafely(JsonObject jsonGroup) { | 272 | public boolean matchesSafely(JsonObject jsonGroup) { |
| 273 | // check id | 273 | // check id |
| 274 | final String jsonId = jsonGroup.get("id").asString(); | 274 | final String jsonId = jsonGroup.get("id").asString(); |
| 275 | - final String groupId = group.id().toString(); | 275 | + final String groupId = group.id().id().toString(); |
| 276 | if (!jsonId.equals(groupId)) { | 276 | if (!jsonId.equals(groupId)) { |
| 277 | - reason = "id " + group.id().toString(); | 277 | + reason = "id " + group.id().id().toString(); |
| 278 | return false; | 278 | return false; |
| 279 | } | 279 | } |
| 280 | 280 | ||
| ... | @@ -356,7 +356,7 @@ public class GroupsResourceTest extends ResourceTest { | ... | @@ -356,7 +356,7 @@ public class GroupsResourceTest extends ResourceTest { |
| 356 | 356 | ||
| 357 | final JsonObject jsonGroup = json.get(jsonGroupIndex).asObject(); | 357 | final JsonObject jsonGroup = json.get(jsonGroupIndex).asObject(); |
| 358 | 358 | ||
| 359 | - final String groupId = group.id().toString(); | 359 | + final String groupId = group.id().id().toString(); |
| 360 | final String jsonGroupId = jsonGroup.get("id").asString(); | 360 | final String jsonGroupId = jsonGroup.get("id").asString(); |
| 361 | if (jsonGroupId.equals(groupId)) { | 361 | if (jsonGroupId.equals(groupId)) { |
| 362 | groupFound = true; | 362 | groupFound = true; |
| ... | @@ -366,7 +366,7 @@ public class GroupsResourceTest extends ResourceTest { | ... | @@ -366,7 +366,7 @@ public class GroupsResourceTest extends ResourceTest { |
| 366 | } | 366 | } |
| 367 | } | 367 | } |
| 368 | if (!groupFound) { | 368 | if (!groupFound) { |
| 369 | - reason = "Group with id " + group.id().toString() + " not found"; | 369 | + reason = "Group with id " + group.id().id().toString() + " not found"; |
| 370 | return false; | 370 | return false; |
| 371 | } else { | 371 | } else { |
| 372 | return true; | 372 | return true; | ... | ... |
-
Please register or login to post a comment