Fixing hash for Intent keys
Change-Id: Ie7807d95b3e58f2e79c6127251ef355b77ba05ff
Showing
2 changed files
with
43 additions
and
13 deletions
... | @@ -26,9 +26,9 @@ import java.util.Objects; | ... | @@ -26,9 +26,9 @@ import java.util.Objects; |
26 | * Key class for Intents. | 26 | * Key class for Intents. |
27 | */ | 27 | */ |
28 | // TODO maybe pull this up to utils | 28 | // TODO maybe pull this up to utils |
29 | -// TODO need to make this classes kryo serializable | 29 | +public abstract class Key { |
30 | -public class Key { | ||
31 | 30 | ||
31 | + //TODO consider making this a HashCode object (worry about performance) | ||
32 | private final long hash; | 32 | private final long hash; |
33 | private static final HashFunction HASH_FN = Hashing.md5(); | 33 | private static final HashFunction HASH_FN = Hashing.md5(); |
34 | 34 | ||
... | @@ -40,15 +40,44 @@ public class Key { | ... | @@ -40,15 +40,44 @@ public class Key { |
40 | return hash; | 40 | return hash; |
41 | } | 41 | } |
42 | 42 | ||
43 | + @Override | ||
44 | + public int hashCode() { | ||
45 | + return (int) (hash() ^ (hash() >>> 32)); | ||
46 | + } | ||
47 | + | ||
48 | + @Override | ||
49 | + public abstract boolean equals(Object obj); | ||
50 | + | ||
51 | + /** | ||
52 | + * Creates a key based on the provided string. | ||
53 | + * <p> | ||
54 | + * Note: Two keys with equal value, but different appId, are not equal. | ||
55 | + * </p> | ||
56 | + * | ||
57 | + * @param key the provided string | ||
58 | + * @param appId application id to associate with this key | ||
59 | + * @return the key for the string | ||
60 | + */ | ||
43 | public static Key of(String key, ApplicationId appId) { | 61 | public static Key of(String key, ApplicationId appId) { |
44 | return new StringKey(key, appId); | 62 | return new StringKey(key, appId); |
45 | } | 63 | } |
46 | 64 | ||
65 | + /** | ||
66 | + * Creates a key based on the provided long. | ||
67 | + * <p> | ||
68 | + * Note: Two keys with equal value, but different appId, are not equal. | ||
69 | + * Also, "10" and 10L are different. | ||
70 | + * </p> | ||
71 | + * | ||
72 | + * @param key the provided long | ||
73 | + * @param appId application id to associate with this key | ||
74 | + * @return the key for the long | ||
75 | + */ | ||
47 | public static Key of(long key, ApplicationId appId) { | 76 | public static Key of(long key, ApplicationId appId) { |
48 | return new LongKey(key, appId); | 77 | return new LongKey(key, appId); |
49 | } | 78 | } |
50 | 79 | ||
51 | - public static final class StringKey extends Key { | 80 | + private static final class StringKey extends Key { |
52 | 81 | ||
53 | private final ApplicationId appId; | 82 | private final ApplicationId appId; |
54 | private final String key; | 83 | private final String key; |
... | @@ -67,9 +96,10 @@ public class Key { | ... | @@ -67,9 +96,10 @@ public class Key { |
67 | return key; | 96 | return key; |
68 | } | 97 | } |
69 | 98 | ||
99 | + // checkstyle requires this | ||
70 | @Override | 100 | @Override |
71 | public int hashCode() { | 101 | public int hashCode() { |
72 | - return key.hashCode(); | 102 | + return super.hashCode(); |
73 | } | 103 | } |
74 | 104 | ||
75 | @Override | 105 | @Override |
... | @@ -87,7 +117,7 @@ public class Key { | ... | @@ -87,7 +117,7 @@ public class Key { |
87 | } | 117 | } |
88 | } | 118 | } |
89 | 119 | ||
90 | - public static final class LongKey extends Key { | 120 | + private static final class LongKey extends Key { |
91 | 121 | ||
92 | private final ApplicationId appId; | 122 | private final ApplicationId appId; |
93 | private final long key; | 123 | private final long key; |
... | @@ -106,9 +136,10 @@ public class Key { | ... | @@ -106,9 +136,10 @@ public class Key { |
106 | return "0x" + Long.toHexString(key); | 136 | return "0x" + Long.toHexString(key); |
107 | } | 137 | } |
108 | 138 | ||
139 | + // checkstyle requires this | ||
109 | @Override | 140 | @Override |
110 | public int hashCode() { | 141 | public int hashCode() { |
111 | - return (int) (key ^ (key >>> 32)); | 142 | + return super.hashCode(); |
112 | } | 143 | } |
113 | 144 | ||
114 | @Override | 145 | @Override |
... | @@ -120,10 +151,10 @@ public class Key { | ... | @@ -120,10 +151,10 @@ public class Key { |
120 | return false; | 151 | return false; |
121 | } | 152 | } |
122 | final LongKey other = (LongKey) obj; | 153 | final LongKey other = (LongKey) obj; |
123 | - return Objects.equals(this.appId, other.appId) && | 154 | + return this.hash() == other.hash() && |
124 | - this.key == other.key; | 155 | + this.key == other.key && |
156 | + Objects.equals(this.appId, other.appId); | ||
125 | } | 157 | } |
126 | - | ||
127 | } | 158 | } |
128 | } | 159 | } |
129 | 160 | ... | ... |
... | @@ -18,7 +18,6 @@ package org.onosproject.store.serializers; | ... | @@ -18,7 +18,6 @@ package org.onosproject.store.serializers; |
18 | import com.google.common.collect.ImmutableList; | 18 | import com.google.common.collect.ImmutableList; |
19 | import com.google.common.collect.ImmutableMap; | 19 | import com.google.common.collect.ImmutableMap; |
20 | import com.google.common.collect.ImmutableSet; | 20 | import com.google.common.collect.ImmutableSet; |
21 | - | ||
22 | import org.onlab.packet.ChassisId; | 21 | import org.onlab.packet.ChassisId; |
23 | import org.onlab.packet.Ip4Address; | 22 | import org.onlab.packet.Ip4Address; |
24 | import org.onlab.packet.Ip4Prefix; | 23 | import org.onlab.packet.Ip4Prefix; |
... | @@ -284,9 +283,9 @@ public final class KryoNamespaces { | ... | @@ -284,9 +283,9 @@ public final class KryoNamespaces { |
284 | FlowRuleBatchEntry.FlowRuleOperation.class, | 283 | FlowRuleBatchEntry.FlowRuleOperation.class, |
285 | IntentId.class, | 284 | IntentId.class, |
286 | IntentState.class, | 285 | IntentState.class, |
287 | - Key.class, | 286 | + //Key.class, is abstract |
288 | - Key.LongKey.class, | 287 | + Key.of(1L, new DefaultApplicationId(0, "bar")).getClass(), //LongKey.class |
289 | - Key.StringKey.class, | 288 | + Key.of("foo", new DefaultApplicationId(0, "bar")).getClass(), //StringKey.class |
290 | Intent.class, | 289 | Intent.class, |
291 | ConnectivityIntent.class, | 290 | ConnectivityIntent.class, |
292 | PathIntent.class, | 291 | PathIntent.class, | ... | ... |
-
Please register or login to post a comment