Committed by
Gerrit Code Review
Fix for JIRA ONOS-4620. Whenever networkconfiguration is deleted even the queued will be removed
Change-Id: I8d7f1a873af90cf86ea34f1a2b1585ef4c3e46e4
Showing
8 changed files
with
149 additions
and
20 deletions
... | @@ -177,4 +177,19 @@ public interface NetworkConfigService | ... | @@ -177,4 +177,19 @@ public interface NetworkConfigService |
177 | * @param <S> type of subject | 177 | * @param <S> type of subject |
178 | */ | 178 | */ |
179 | <S> void removeConfig(String subjectClassKey, S subject, String configKey); | 179 | <S> void removeConfig(String subjectClassKey, S subject, String configKey); |
180 | + | ||
181 | + /** | ||
182 | + * Clears the configuration including queued based on the subject. | ||
183 | + * If does not exists this call has no effect. | ||
184 | + * | ||
185 | + * @param subject configuration subject | ||
186 | + */ | ||
187 | + <S> void removeConfig(S subject); | ||
188 | + | ||
189 | + /** | ||
190 | + * Clears the complete configuration including queued. | ||
191 | + * If does not exists this call has no effect. | ||
192 | + * | ||
193 | + */ | ||
194 | + <S> void removeConfig(); | ||
180 | } | 195 | } | ... | ... |
... | @@ -151,4 +151,19 @@ public interface NetworkConfigStore extends Store<NetworkConfigEvent, NetworkCon | ... | @@ -151,4 +151,19 @@ public interface NetworkConfigStore extends Store<NetworkConfigEvent, NetworkCon |
151 | */ | 151 | */ |
152 | <S> void clearQueuedConfig(S subject, String configKey); | 152 | <S> void clearQueuedConfig(S subject, String configKey); |
153 | 153 | ||
154 | + /** | ||
155 | + * Clears the configuration based on the subject including queued. | ||
156 | + * If does not exists this call has no effect. | ||
157 | + * | ||
158 | + * @param subject configuration subject | ||
159 | + */ | ||
160 | + <S> void clearConfig(S subject); | ||
161 | + | ||
162 | + /** | ||
163 | + * Clears the complete configuration including queued. | ||
164 | + * If does not exists this call has no effect. | ||
165 | + * | ||
166 | + */ | ||
167 | + <S> void clearConfig(); | ||
168 | + | ||
154 | } | 169 | } | ... | ... |
... | @@ -95,4 +95,12 @@ public class NetworkConfigServiceAdapter implements NetworkConfigService { | ... | @@ -95,4 +95,12 @@ public class NetworkConfigServiceAdapter implements NetworkConfigService { |
95 | @Override | 95 | @Override |
96 | public void removeListener(NetworkConfigListener listener) { | 96 | public void removeListener(NetworkConfigListener listener) { |
97 | } | 97 | } |
98 | + | ||
99 | + @Override | ||
100 | + public <S> void removeConfig(S subject) { | ||
101 | + } | ||
102 | + | ||
103 | + @Override | ||
104 | + public <S> void removeConfig() { | ||
105 | + } | ||
98 | } | 106 | } | ... | ... |
... | @@ -261,6 +261,18 @@ public class NetworkConfigManager | ... | @@ -261,6 +261,18 @@ public class NetworkConfigManager |
261 | } | 261 | } |
262 | } | 262 | } |
263 | 263 | ||
264 | + @Override | ||
265 | + public <S> void removeConfig(S subject) { | ||
266 | + checkPermission(CONFIG_WRITE); | ||
267 | + store.clearConfig(subject); | ||
268 | + } | ||
269 | + | ||
270 | + @Override | ||
271 | + public <S> void removeConfig() { | ||
272 | + checkPermission(CONFIG_WRITE); | ||
273 | + store.clearConfig(); | ||
274 | + } | ||
275 | + | ||
264 | // Auxiliary store delegate to receive notification about changes in | 276 | // Auxiliary store delegate to receive notification about changes in |
265 | // the network configuration store state - by the store itself. | 277 | // the network configuration store state - by the store itself. |
266 | private class InternalStoreDelegate implements NetworkConfigStoreDelegate { | 278 | private class InternalStoreDelegate implements NetworkConfigStoreDelegate { | ... | ... |
... | @@ -239,4 +239,43 @@ public class NetworkConfigManagerTest { | ... | @@ -239,4 +239,43 @@ public class NetworkConfigManagerTest { |
239 | assertThat(configService.getSubjectFactory(String.class), notNullValue()); | 239 | assertThat(configService.getSubjectFactory(String.class), notNullValue()); |
240 | assertThat(configService.getSubjectFactory("key1"), notNullValue()); | 240 | assertThat(configService.getSubjectFactory("key1"), notNullValue()); |
241 | } | 241 | } |
242 | + | ||
243 | + /** | ||
244 | + * Tests creation, query and removal of a configuration including queued. | ||
245 | + */ | ||
246 | + @Test | ||
247 | + public void testRemoveConfig() { | ||
248 | + | ||
249 | + assertThat(configService.getSubjectFactory(String.class), nullValue()); | ||
250 | + assertThat(configService.getSubjectFactory("key"), nullValue()); | ||
251 | + | ||
252 | + registry.registerConfigFactory(config1Factory); | ||
253 | + registry.registerConfigFactory(config2Factory); | ||
254 | + configService.applyConfig("configKey", BasicConfig1.class, new ObjectMapper().createObjectNode()); | ||
255 | + | ||
256 | + configService.applyConfig("key1", "key", "config1", new ObjectMapper().createObjectNode()); | ||
257 | + configService.applyConfig("key1", "keyxx", "config3", new ObjectMapper().createObjectNode()); | ||
258 | + configService.applyConfig("key2", "key1", "config4", new ObjectMapper().createObjectNode()); | ||
259 | + | ||
260 | + configService.removeConfig(); | ||
261 | + | ||
262 | + Set<String> subjects = configService.getSubjects(factory1.subjectClass()); | ||
263 | + assertThat(subjects.size(), is(0)); | ||
264 | + | ||
265 | + Set<String> subjects2 = configService.getSubjects(factory2.subjectClass()); | ||
266 | + assertThat(subjects2.size(), is(0)); | ||
267 | + | ||
268 | + configService.applyConfig("key1", "key", "config1", new ObjectMapper().createObjectNode()); | ||
269 | + configService.applyConfig("key1", "keyxx", "config3", new ObjectMapper().createObjectNode()); | ||
270 | + configService.applyConfig("key1", "key1", "config4", new ObjectMapper().createObjectNode()); | ||
271 | + | ||
272 | + @SuppressWarnings("unchecked") | ||
273 | + Set<String> configs = configService.getSubjects( | ||
274 | + configService.getSubjectFactory("key1").subjectClass()); | ||
275 | + | ||
276 | + configs.forEach(c -> configService.removeConfig(c)); | ||
277 | + Set<String> newConfig1 = configService.getSubjects(factory1.subjectClass()); | ||
278 | + | ||
279 | + assertThat(newConfig1, notNullValue()); | ||
280 | + } | ||
242 | } | 281 | } | ... | ... |
... | @@ -284,6 +284,24 @@ public class DistributedNetworkConfigStore | ... | @@ -284,6 +284,24 @@ public class DistributedNetworkConfigStore |
284 | configs.remove(key(subject, configKey)); | 284 | configs.remove(key(subject, configKey)); |
285 | } | 285 | } |
286 | 286 | ||
287 | + @Override | ||
288 | + public <S> void clearConfig(S subject) { | ||
289 | + ImmutableSet.copyOf(configs.keySet()).forEach(k -> { | ||
290 | + if (Objects.equals(subject, k.subject) && delegate != null) { | ||
291 | + configs.remove(k); | ||
292 | + } | ||
293 | + }); | ||
294 | + } | ||
295 | + | ||
296 | + @Override | ||
297 | + public <S> void clearConfig() { | ||
298 | + ImmutableSet.copyOf(configs.keySet()).forEach(k -> { | ||
299 | + if (delegate != null) { | ||
300 | + configs.remove(k); | ||
301 | + } | ||
302 | + }); | ||
303 | + } | ||
304 | + | ||
287 | /** | 305 | /** |
288 | * Produces a config from the specified subject, config class and raw JSON. | 306 | * Produces a config from the specified subject, config class and raw JSON. |
289 | * | 307 | * | ... | ... |
... | @@ -31,6 +31,9 @@ import static org.hamcrest.Matchers.notNullValue; | ... | @@ -31,6 +31,9 @@ import static org.hamcrest.Matchers.notNullValue; |
31 | import static org.hamcrest.Matchers.nullValue; | 31 | import static org.hamcrest.Matchers.nullValue; |
32 | import static org.junit.Assert.assertThat; | 32 | import static org.junit.Assert.assertThat; |
33 | 33 | ||
34 | +import java.util.Set; | ||
35 | + | ||
36 | + | ||
34 | public class DistributedNetworkConfigStoreTest { | 37 | public class DistributedNetworkConfigStoreTest { |
35 | private DistributedNetworkConfigStore configStore; | 38 | private DistributedNetworkConfigStore configStore; |
36 | 39 | ||
... | @@ -203,4 +206,33 @@ public class DistributedNetworkConfigStoreTest { | ... | @@ -203,4 +206,33 @@ public class DistributedNetworkConfigStoreTest { |
203 | assertThat(configStore.getSubjects(Integer.class, BasicIntConfig.class), hasSize(1)); | 206 | assertThat(configStore.getSubjects(Integer.class, BasicIntConfig.class), hasSize(1)); |
204 | assertThat(configStore.getSubjects(Integer.class), hasSize(1)); | 207 | assertThat(configStore.getSubjects(Integer.class), hasSize(1)); |
205 | } | 208 | } |
209 | + | ||
210 | + /** | ||
211 | + * Tests removal of config including queued. | ||
212 | + */ | ||
213 | + @Test | ||
214 | + public void testRemoveConfig() { | ||
215 | + | ||
216 | + configStore.addConfigFactory(new MockConfigFactory(BasicConfig.class, "config1")); | ||
217 | + configStore.queueConfig("subject", "config2", new ObjectMapper().createObjectNode()); | ||
218 | + configStore.queueConfig(123, "config2", new ObjectMapper().createObjectNode()); | ||
219 | + configStore.applyConfig("subject1", BasicConfig.class, new ObjectMapper().createObjectNode()); | ||
220 | + | ||
221 | + configStore.clearConfig(); | ||
222 | + | ||
223 | + Set<String> subjects = configStore.getSubjects(String.class); | ||
224 | + assertThat(subjects.size(), is(0)); | ||
225 | + | ||
226 | + configStore.addConfigFactory(new MockConfigFactory(BasicConfig.class, "config1")); | ||
227 | + configStore.queueConfig("subject", "config3", new ObjectMapper().createObjectNode()); | ||
228 | + configStore.queueConfig(123, "config3", new ObjectMapper().createObjectNode()); | ||
229 | + configStore.applyConfig("subject1", BasicConfig.class, new ObjectMapper().createObjectNode()); | ||
230 | + | ||
231 | + Set<String> configs = configStore.getSubjects(String.class); | ||
232 | + | ||
233 | + configs.forEach(c -> configStore.clearConfig(c)); | ||
234 | + Set<String> newConfig1 = configStore.getSubjects(String.class); | ||
235 | + | ||
236 | + assertThat(newConfig1, notNullValue()); | ||
237 | + } | ||
206 | } | 238 | } | ... | ... |
... | @@ -294,28 +294,22 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -294,28 +294,22 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
294 | @SuppressWarnings("unchecked") | 294 | @SuppressWarnings("unchecked") |
295 | public Response delete() { | 295 | public Response delete() { |
296 | NetworkConfigService service = get(NetworkConfigService.class); | 296 | NetworkConfigService service = get(NetworkConfigService.class); |
297 | - service.getSubjectClasses() | 297 | + service.removeConfig(); |
298 | - .forEach(subjectClass -> service.getSubjects(subjectClass) | 298 | + return Response.ok().build(); |
299 | - .forEach(subject -> service.getConfigs(subject) | ||
300 | - .forEach(config -> service.removeConfig(subject, config.getClass())))); | ||
301 | - return Response.noContent().build(); | ||
302 | } | 299 | } |
303 | 300 | ||
304 | /** | 301 | /** |
305 | * Clear all network configurations for a subject class. | 302 | * Clear all network configurations for a subject class. |
306 | * | 303 | * |
307 | * @param subjectClassKey subject class key | 304 | * @param subjectClassKey subject class key |
308 | - * @return 204 NO CONTENT | ||
309 | */ | 305 | */ |
310 | @DELETE | 306 | @DELETE |
311 | @Path("{subjectClassKey}") | 307 | @Path("{subjectClassKey}") |
312 | @SuppressWarnings("unchecked") | 308 | @SuppressWarnings("unchecked") |
313 | - public Response delete(@PathParam("subjectClassKey") String subjectClassKey) { | 309 | + public void delete(@PathParam("subjectClassKey") String subjectClassKey) { |
314 | NetworkConfigService service = get(NetworkConfigService.class); | 310 | NetworkConfigService service = get(NetworkConfigService.class); |
315 | service.getSubjects(service.getSubjectFactory(subjectClassKey).subjectClass()) | 311 | service.getSubjects(service.getSubjectFactory(subjectClassKey).subjectClass()) |
316 | - .forEach(subject -> service.getConfigs(subject) | 312 | + .forEach(subject -> service.removeConfig(subject)); |
317 | - .forEach(config -> service.removeConfig(subject, config.getClass()))); | ||
318 | - return Response.noContent().build(); | ||
319 | } | 313 | } |
320 | 314 | ||
321 | /** | 315 | /** |
... | @@ -323,17 +317,14 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -323,17 +317,14 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
323 | * | 317 | * |
324 | * @param subjectClassKey subjectKey class key | 318 | * @param subjectClassKey subjectKey class key |
325 | * @param subjectKey subjectKey key | 319 | * @param subjectKey subjectKey key |
326 | - * @return 204 NO CONTENT | ||
327 | */ | 320 | */ |
328 | @DELETE | 321 | @DELETE |
329 | @Path("{subjectClassKey}/{subjectKey}") | 322 | @Path("{subjectClassKey}/{subjectKey}") |
330 | @SuppressWarnings("unchecked") | 323 | @SuppressWarnings("unchecked") |
331 | - public Response delete(@PathParam("subjectClassKey") String subjectClassKey, | 324 | + public void delete(@PathParam("subjectClassKey") String subjectClassKey, |
332 | @PathParam("subjectKey") String subjectKey) { | 325 | @PathParam("subjectKey") String subjectKey) { |
333 | NetworkConfigService service = get(NetworkConfigService.class); | 326 | NetworkConfigService service = get(NetworkConfigService.class); |
334 | - Object s = service.getSubjectFactory(subjectClassKey).createSubject(subjectKey); | 327 | + service.removeConfig(subjectKey); |
335 | - service.getConfigs(s).forEach(c -> service.removeConfig(s, c.getClass())); | ||
336 | - return Response.noContent().build(); | ||
337 | } | 328 | } |
338 | 329 | ||
339 | /** | 330 | /** |
... | @@ -342,18 +333,17 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -342,18 +333,17 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
342 | * @param subjectClassKey subjectKey class key | 333 | * @param subjectClassKey subjectKey class key |
343 | * @param subjectKey subjectKey key | 334 | * @param subjectKey subjectKey key |
344 | * @param configKey configuration class key | 335 | * @param configKey configuration class key |
345 | - * @return 204 NO CONTENT | ||
346 | */ | 336 | */ |
347 | @DELETE | 337 | @DELETE |
348 | @Path("{subjectClassKey}/{subjectKey}/{configKey}") | 338 | @Path("{subjectClassKey}/{subjectKey}/{configKey}") |
349 | @SuppressWarnings("unchecked") | 339 | @SuppressWarnings("unchecked") |
350 | - public Response delete(@PathParam("subjectClassKey") String subjectClassKey, | 340 | + public void delete(@PathParam("subjectClassKey") String subjectClassKey, |
351 | @PathParam("subjectKey") String subjectKey, | 341 | @PathParam("subjectKey") String subjectKey, |
352 | @PathParam("configKey") String configKey) { | 342 | @PathParam("configKey") String configKey) { |
353 | NetworkConfigService service = get(NetworkConfigService.class); | 343 | NetworkConfigService service = get(NetworkConfigService.class); |
354 | - service.removeConfig(service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), | 344 | + service.removeConfig(subjectClassKey, |
355 | - service.getConfigClass(subjectClassKey, configKey)); | 345 | + service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), |
356 | - return Response.noContent().build(); | 346 | + configKey); |
357 | } | 347 | } |
358 | 348 | ||
359 | } | 349 | } | ... | ... |
-
Please register or login to post a comment