Committed by
Ray Milkey
Implement copyData as copy constructor on IntentData.
Cleaned up javadocs. Change-Id: I90e6350244991d4f30180fe501fec9e6fd180d43
Showing
3 changed files
with
93 additions
and
39 deletions
... | @@ -23,13 +23,15 @@ import org.onosproject.store.Timestamp; | ... | @@ -23,13 +23,15 @@ import org.onosproject.store.Timestamp; |
23 | import java.util.List; | 23 | import java.util.List; |
24 | import java.util.Objects; | 24 | import java.util.Objects; |
25 | 25 | ||
26 | +import static com.google.common.base.Preconditions.checkNotNull; | ||
27 | + | ||
26 | /** | 28 | /** |
27 | * A wrapper class that contains an intents, its state, and other metadata for | 29 | * A wrapper class that contains an intents, its state, and other metadata for |
28 | * internal use. | 30 | * internal use. |
29 | */ | 31 | */ |
30 | public class IntentData { //FIXME need to make this "immutable" | 32 | public class IntentData { //FIXME need to make this "immutable" |
31 | // manager should be able to mutate a local copy while processing | 33 | // manager should be able to mutate a local copy while processing |
32 | - private Intent intent; | 34 | + private final Intent intent; |
33 | 35 | ||
34 | private IntentState state; | 36 | private IntentState state; |
35 | private Timestamp version; | 37 | private Timestamp version; |
... | @@ -37,34 +39,77 @@ public class IntentData { //FIXME need to make this "immutable" | ... | @@ -37,34 +39,77 @@ public class IntentData { //FIXME need to make this "immutable" |
37 | 39 | ||
38 | private List<Intent> installables; | 40 | private List<Intent> installables; |
39 | 41 | ||
42 | + /** | ||
43 | + * Creates a new intent data object. | ||
44 | + * | ||
45 | + * @param intent intent this metadata references | ||
46 | + * @param state intent state | ||
47 | + * @param version version of the intent for this key | ||
48 | + */ | ||
40 | public IntentData(Intent intent, IntentState state, Timestamp version) { | 49 | public IntentData(Intent intent, IntentState state, Timestamp version) { |
41 | this.intent = intent; | 50 | this.intent = intent; |
42 | this.state = state; | 51 | this.state = state; |
43 | this.version = version; | 52 | this.version = version; |
44 | } | 53 | } |
45 | 54 | ||
55 | + /** | ||
56 | + * Copy constructor. | ||
57 | + * | ||
58 | + * @param intentData intent data to copy | ||
59 | + */ | ||
60 | + public IntentData(IntentData intentData) { | ||
61 | + checkNotNull(intentData); | ||
62 | + | ||
63 | + intent = intentData.intent; | ||
64 | + state = intentData.state; | ||
65 | + version = intentData.version; | ||
66 | + origin = intentData.origin; | ||
67 | + installables = intentData.installables; | ||
68 | + } | ||
69 | + | ||
46 | // kryo constructor | 70 | // kryo constructor |
47 | protected IntentData() { | 71 | protected IntentData() { |
72 | + intent = null; | ||
48 | } | 73 | } |
49 | 74 | ||
75 | + /** | ||
76 | + * Returns the intent this metadata references. | ||
77 | + * | ||
78 | + * @return intent | ||
79 | + */ | ||
50 | public Intent intent() { | 80 | public Intent intent() { |
51 | return intent; | 81 | return intent; |
52 | } | 82 | } |
53 | 83 | ||
84 | + /** | ||
85 | + * Returns the state of the intent. | ||
86 | + * | ||
87 | + * @return intent state | ||
88 | + */ | ||
54 | public IntentState state() { | 89 | public IntentState state() { |
55 | return state; | 90 | return state; |
56 | } | 91 | } |
57 | 92 | ||
93 | + /** | ||
94 | + * Returns the intent key. | ||
95 | + * | ||
96 | + * @return intent key | ||
97 | + */ | ||
58 | public Key key() { | 98 | public Key key() { |
59 | return intent.key(); | 99 | return intent.key(); |
60 | } | 100 | } |
61 | 101 | ||
102 | + /** | ||
103 | + * Returns the version of the intent for this key. | ||
104 | + * | ||
105 | + * @return intent version | ||
106 | + */ | ||
62 | public Timestamp version() { | 107 | public Timestamp version() { |
63 | return version; | 108 | return version; |
64 | } | 109 | } |
65 | 110 | ||
66 | /** | 111 | /** |
67 | - * Sets the origin, which is the node that created the instance. | 112 | + * Sets the origin, which is the node that created the intent. |
68 | * | 113 | * |
69 | * @param origin origin instance | 114 | * @param origin origin instance |
70 | */ | 115 | */ |
... | @@ -72,10 +117,20 @@ public class IntentData { //FIXME need to make this "immutable" | ... | @@ -72,10 +117,20 @@ public class IntentData { //FIXME need to make this "immutable" |
72 | this.origin = origin; | 117 | this.origin = origin; |
73 | } | 118 | } |
74 | 119 | ||
120 | + /** | ||
121 | + * Returns the origin node that created this intent. | ||
122 | + * | ||
123 | + * @return origin node ID | ||
124 | + */ | ||
75 | public NodeId origin() { | 125 | public NodeId origin() { |
76 | return origin; | 126 | return origin; |
77 | } | 127 | } |
78 | 128 | ||
129 | + /** | ||
130 | + * Updates the state of the intent to the given new state. | ||
131 | + * | ||
132 | + * @param newState new state of the intent | ||
133 | + */ | ||
79 | public void setState(IntentState newState) { | 134 | public void setState(IntentState newState) { |
80 | this.state = newState; | 135 | this.state = newState; |
81 | } | 136 | } |
... | @@ -94,10 +149,20 @@ public class IntentData { //FIXME need to make this "immutable" | ... | @@ -94,10 +149,20 @@ public class IntentData { //FIXME need to make this "immutable" |
94 | this.version = version; | 149 | this.version = version; |
95 | } | 150 | } |
96 | 151 | ||
152 | + /** | ||
153 | + * Sets the intent installables to the given list of intents. | ||
154 | + * | ||
155 | + * @param installables list of installables for this intent | ||
156 | + */ | ||
97 | public void setInstallables(List<Intent> installables) { | 157 | public void setInstallables(List<Intent> installables) { |
98 | this.installables = ImmutableList.copyOf(installables); | 158 | this.installables = ImmutableList.copyOf(installables); |
99 | } | 159 | } |
100 | 160 | ||
161 | + /** | ||
162 | + * Returns the installables associated with this intent. | ||
163 | + * | ||
164 | + * @return list of installable intents | ||
165 | + */ | ||
101 | public List<Intent> installables() { | 166 | public List<Intent> installables() { |
102 | return installables; | 167 | return installables; |
103 | } | 168 | } |
... | @@ -120,12 +185,14 @@ public class IntentData { //FIXME need to make this "immutable" | ... | @@ -120,12 +185,14 @@ public class IntentData { //FIXME need to make this "immutable" |
120 | && Objects.equals(this.version, other.version); | 185 | && Objects.equals(this.version, other.version); |
121 | } | 186 | } |
122 | 187 | ||
188 | + @Override | ||
123 | public String toString() { | 189 | public String toString() { |
124 | return MoreObjects.toStringHelper(getClass()) | 190 | return MoreObjects.toStringHelper(getClass()) |
125 | .add("key", key()) | 191 | .add("key", key()) |
126 | .add("state", state()) | 192 | .add("state", state()) |
127 | .add("version", version()) | 193 | .add("version", version()) |
128 | .add("intent", intent()) | 194 | .add("intent", intent()) |
195 | + .add("origin", origin()) | ||
129 | .add("installables", installables()) | 196 | .add("installables", installables()) |
130 | .toString(); | 197 | .toString(); |
131 | } | 198 | } | ... | ... |
... | @@ -51,6 +51,7 @@ import java.util.List; | ... | @@ -51,6 +51,7 @@ import java.util.List; |
51 | import java.util.Objects; | 51 | import java.util.Objects; |
52 | import java.util.stream.Collectors; | 52 | import java.util.stream.Collectors; |
53 | 53 | ||
54 | +import static com.google.common.base.Preconditions.checkNotNull; | ||
54 | import static org.onosproject.net.intent.IntentState.*; | 55 | import static org.onosproject.net.intent.IntentState.*; |
55 | import static org.slf4j.LoggerFactory.getLogger; | 56 | import static org.slf4j.LoggerFactory.getLogger; |
56 | 57 | ||
... | @@ -149,20 +150,6 @@ public class GossipIntentStore | ... | @@ -149,20 +150,6 @@ public class GossipIntentStore |
149 | return null; | 150 | return null; |
150 | } | 151 | } |
151 | 152 | ||
152 | - private IntentData copyData(IntentData original) { | ||
153 | - if (original == null) { | ||
154 | - return null; | ||
155 | - } | ||
156 | - IntentData result = | ||
157 | - new IntentData(original.intent(), original.state(), original.version()); | ||
158 | - | ||
159 | - if (original.installables() != null) { | ||
160 | - result.setInstallables(original.installables()); | ||
161 | - } | ||
162 | - result.setOrigin(original.origin()); | ||
163 | - return result; | ||
164 | - } | ||
165 | - | ||
166 | /** | 153 | /** |
167 | * Determines whether an intent data update is allowed. The update must | 154 | * Determines whether an intent data update is allowed. The update must |
168 | * either have a higher version than the current data, or the state | 155 | * either have a higher version than the current data, or the state |
... | @@ -239,6 +226,8 @@ public class GossipIntentStore | ... | @@ -239,6 +226,8 @@ public class GossipIntentStore |
239 | 226 | ||
240 | @Override | 227 | @Override |
241 | public void write(IntentData newData) { | 228 | public void write(IntentData newData) { |
229 | + checkNotNull(newData); | ||
230 | + | ||
242 | IntentData currentData = currentMap.get(newData.key()); | 231 | IntentData currentData = currentMap.get(newData.key()); |
243 | if (isUpdateAcceptable(currentData, newData)) { | 232 | if (isUpdateAcceptable(currentData, newData)) { |
244 | // Only the master is modifying the current state. Therefore assume | 233 | // Only the master is modifying the current state. Therefore assume |
... | @@ -246,13 +235,11 @@ public class GossipIntentStore | ... | @@ -246,13 +235,11 @@ public class GossipIntentStore |
246 | if (newData.state() == PURGE_REQ) { | 235 | if (newData.state() == PURGE_REQ) { |
247 | currentMap.remove(newData.key(), newData); | 236 | currentMap.remove(newData.key(), newData); |
248 | } else { | 237 | } else { |
249 | - currentMap.put(newData.key(), copyData(newData)); | 238 | + currentMap.put(newData.key(), new IntentData(newData)); |
250 | } | 239 | } |
251 | 240 | ||
252 | // if current.put succeeded | 241 | // if current.put succeeded |
253 | pendingMap.remove(newData.key(), newData); | 242 | pendingMap.remove(newData.key(), newData); |
254 | - } else { | ||
255 | - log.debug("not writing update: current {}, new {}", currentData, newData); | ||
256 | } | 243 | } |
257 | } | 244 | } |
258 | 245 | ||
... | @@ -307,16 +294,22 @@ public class GossipIntentStore | ... | @@ -307,16 +294,22 @@ public class GossipIntentStore |
307 | 294 | ||
308 | @Override | 295 | @Override |
309 | public IntentData getIntentData(Key key) { | 296 | public IntentData getIntentData(Key key) { |
310 | - return copyData(currentMap.get(key)); | 297 | + IntentData current = currentMap.get(key); |
298 | + if (current == null) { | ||
299 | + return null; | ||
300 | + } | ||
301 | + return new IntentData(current); | ||
311 | } | 302 | } |
312 | 303 | ||
313 | @Override | 304 | @Override |
314 | public void addPending(IntentData data) { | 305 | public void addPending(IntentData data) { |
306 | + checkNotNull(data); | ||
307 | + | ||
315 | if (data.version() == null) { | 308 | if (data.version() == null) { |
316 | data.setVersion(new WallClockTimestamp()); | 309 | data.setVersion(new WallClockTimestamp()); |
317 | } | 310 | } |
318 | data.setOrigin(clusterService.getLocalNode().id()); | 311 | data.setOrigin(clusterService.getLocalNode().id()); |
319 | - pendingMap.put(data.key(), copyData(data)); | 312 | + pendingMap.put(data.key(), new IntentData(data)); |
320 | } | 313 | } |
321 | 314 | ||
322 | @Override | 315 | @Override |
... | @@ -361,8 +354,7 @@ public class GossipIntentStore | ... | @@ -361,8 +354,7 @@ public class GossipIntentStore |
361 | // some work. | 354 | // some work. |
362 | if (isMaster(event.value().intent().key())) { | 355 | if (isMaster(event.value().intent().key())) { |
363 | if (delegate != null) { | 356 | if (delegate != null) { |
364 | - log.debug("processing {}", event.key()); | 357 | + delegate.process(new IntentData(event.value())); |
365 | - delegate.process(copyData(event.value())); | ||
366 | } | 358 | } |
367 | } | 359 | } |
368 | 360 | ... | ... |
... | @@ -38,6 +38,9 @@ import static com.google.common.base.Preconditions.checkNotNull; | ... | @@ -38,6 +38,9 @@ import static com.google.common.base.Preconditions.checkNotNull; |
38 | import static org.onosproject.net.intent.IntentState.*; | 38 | import static org.onosproject.net.intent.IntentState.*; |
39 | import static org.slf4j.LoggerFactory.getLogger; | 39 | import static org.slf4j.LoggerFactory.getLogger; |
40 | 40 | ||
41 | +/** | ||
42 | + * Simple single-instance implementation of the intent store. | ||
43 | + */ | ||
41 | @Component(immediate = true) | 44 | @Component(immediate = true) |
42 | @Service | 45 | @Service |
43 | public class SimpleIntentStore | 46 | public class SimpleIntentStore |
... | @@ -49,19 +52,6 @@ public class SimpleIntentStore | ... | @@ -49,19 +52,6 @@ public class SimpleIntentStore |
49 | private final Map<Key, IntentData> current = Maps.newConcurrentMap(); | 52 | private final Map<Key, IntentData> current = Maps.newConcurrentMap(); |
50 | private final Map<Key, IntentData> pending = Maps.newConcurrentMap(); | 53 | private final Map<Key, IntentData> pending = Maps.newConcurrentMap(); |
51 | 54 | ||
52 | - private IntentData copyData(IntentData original) { | ||
53 | - if (original == null) { | ||
54 | - return null; | ||
55 | - } | ||
56 | - IntentData result = | ||
57 | - new IntentData(original.intent(), original.state(), original.version()); | ||
58 | - | ||
59 | - if (original.installables() != null) { | ||
60 | - result.setInstallables(original.installables()); | ||
61 | - } | ||
62 | - return result; | ||
63 | - } | ||
64 | - | ||
65 | @Activate | 55 | @Activate |
66 | public void activate() { | 56 | public void activate() { |
67 | log.info("Started"); | 57 | log.info("Started"); |
... | @@ -99,7 +89,6 @@ public class SimpleIntentStore | ... | @@ -99,7 +89,6 @@ public class SimpleIntentStore |
99 | return null; | 89 | return null; |
100 | } | 90 | } |
101 | 91 | ||
102 | - | ||
103 | /** | 92 | /** |
104 | * Determines whether an intent data update is allowed. The update must | 93 | * Determines whether an intent data update is allowed. The update must |
105 | * either have a higher version than the current data, or the state | 94 | * either have a higher version than the current data, or the state |
... | @@ -174,6 +163,8 @@ public class SimpleIntentStore | ... | @@ -174,6 +163,8 @@ public class SimpleIntentStore |
174 | 163 | ||
175 | @Override | 164 | @Override |
176 | public void write(IntentData newData) { | 165 | public void write(IntentData newData) { |
166 | + checkNotNull(newData); | ||
167 | + | ||
177 | synchronized (this) { | 168 | synchronized (this) { |
178 | // TODO this could be refactored/cleaned up | 169 | // TODO this could be refactored/cleaned up |
179 | IntentData currentData = current.get(newData.key()); | 170 | IntentData currentData = current.get(newData.key()); |
... | @@ -183,7 +174,7 @@ public class SimpleIntentStore | ... | @@ -183,7 +174,7 @@ public class SimpleIntentStore |
183 | if (pendingData.state() == PURGE_REQ) { | 174 | if (pendingData.state() == PURGE_REQ) { |
184 | current.remove(newData.key(), newData); | 175 | current.remove(newData.key(), newData); |
185 | } else { | 176 | } else { |
186 | - current.put(newData.key(), copyData(newData)); | 177 | + current.put(newData.key(), new IntentData(newData)); |
187 | } | 178 | } |
188 | 179 | ||
189 | if (pendingData != null | 180 | if (pendingData != null |
... | @@ -219,7 +210,11 @@ public class SimpleIntentStore | ... | @@ -219,7 +210,11 @@ public class SimpleIntentStore |
219 | 210 | ||
220 | @Override | 211 | @Override |
221 | public IntentData getIntentData(Key key) { | 212 | public IntentData getIntentData(Key key) { |
222 | - return copyData(current.get(key)); | 213 | + IntentData currentData = current.get(key); |
214 | + if (currentData == null) { | ||
215 | + return null; | ||
216 | + } | ||
217 | + return new IntentData(currentData); | ||
223 | } | 218 | } |
224 | 219 | ||
225 | @Override | 220 | @Override | ... | ... |
-
Please register or login to post a comment