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
145 additions
and
10 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 | } | ... | ... |
| ... | @@ -253,6 +253,24 @@ public class DistributedNetworkConfigStore | ... | @@ -253,6 +253,24 @@ public class DistributedNetworkConfigStore |
| 253 | configs.remove(key(subject, configKey)); | 253 | configs.remove(key(subject, configKey)); |
| 254 | } | 254 | } |
| 255 | 255 | ||
| 256 | + @Override | ||
| 257 | + public <S> void clearConfig(S subject) { | ||
| 258 | + ImmutableSet.copyOf(configs.keySet()).forEach(k -> { | ||
| 259 | + if (Objects.equals(subject, k.subject) && delegate != null) { | ||
| 260 | + configs.remove(k); | ||
| 261 | + } | ||
| 262 | + }); | ||
| 263 | + } | ||
| 264 | + | ||
| 265 | + @Override | ||
| 266 | + public <S> void clearConfig() { | ||
| 267 | + ImmutableSet.copyOf(configs.keySet()).forEach(k -> { | ||
| 268 | + if (delegate != null) { | ||
| 269 | + configs.remove(k); | ||
| 270 | + } | ||
| 271 | + }); | ||
| 272 | + } | ||
| 273 | + | ||
| 256 | /** | 274 | /** |
| 257 | * Produces a config from the specified subject, config class and raw JSON. | 275 | * Produces a config from the specified subject, config class and raw JSON. |
| 258 | * | 276 | * | ... | ... |
| ... | @@ -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 | } | ... | ... |
| ... | @@ -297,10 +297,7 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -297,10 +297,7 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 297 | @SuppressWarnings("unchecked") | 297 | @SuppressWarnings("unchecked") |
| 298 | public Response delete() { | 298 | public Response delete() { |
| 299 | NetworkConfigService service = get(NetworkConfigService.class); | 299 | NetworkConfigService service = get(NetworkConfigService.class); |
| 300 | - service.getSubjectClasses() | 300 | + service.removeConfig(); |
| 301 | - .forEach(subjectClass -> service.getSubjects(subjectClass) | ||
| 302 | - .forEach(subject -> service.getConfigs(subject) | ||
| 303 | - .forEach(config -> service.removeConfig(subject, config.getClass())))); | ||
| 304 | return Response.ok().build(); | 301 | return Response.ok().build(); |
| 305 | } | 302 | } |
| 306 | 303 | ||
| ... | @@ -315,8 +312,7 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -315,8 +312,7 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 315 | public void delete(@PathParam("subjectClassKey") String subjectClassKey) { | 312 | public void delete(@PathParam("subjectClassKey") String subjectClassKey) { |
| 316 | NetworkConfigService service = get(NetworkConfigService.class); | 313 | NetworkConfigService service = get(NetworkConfigService.class); |
| 317 | service.getSubjects(service.getSubjectFactory(subjectClassKey).subjectClass()) | 314 | service.getSubjects(service.getSubjectFactory(subjectClassKey).subjectClass()) |
| 318 | - .forEach(subject -> service.getConfigs(subject) | 315 | + .forEach(subject -> service.removeConfig(subject)); |
| 319 | - .forEach(config -> service.removeConfig(subject, config.getClass()))); | ||
| 320 | } | 316 | } |
| 321 | 317 | ||
| 322 | /** | 318 | /** |
| ... | @@ -331,8 +327,7 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -331,8 +327,7 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 331 | public void delete(@PathParam("subjectClassKey") String subjectClassKey, | 327 | public void delete(@PathParam("subjectClassKey") String subjectClassKey, |
| 332 | @PathParam("subjectKey") String subjectKey) { | 328 | @PathParam("subjectKey") String subjectKey) { |
| 333 | NetworkConfigService service = get(NetworkConfigService.class); | 329 | NetworkConfigService service = get(NetworkConfigService.class); |
| 334 | - Object s = service.getSubjectFactory(subjectClassKey).createSubject(subjectKey); | 330 | + service.removeConfig(subjectKey); |
| 335 | - service.getConfigs(s).forEach(c -> service.removeConfig(s, c.getClass())); | ||
| 336 | } | 331 | } |
| 337 | 332 | ||
| 338 | /** | 333 | /** |
| ... | @@ -349,8 +344,9 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -349,8 +344,9 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 349 | @PathParam("subjectKey") String subjectKey, | 344 | @PathParam("subjectKey") String subjectKey, |
| 350 | @PathParam("configKey") String configKey) { | 345 | @PathParam("configKey") String configKey) { |
| 351 | NetworkConfigService service = get(NetworkConfigService.class); | 346 | NetworkConfigService service = get(NetworkConfigService.class); |
| 352 | - service.removeConfig(service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), | 347 | + service.removeConfig(subjectClassKey, |
| 353 | - service.getConfigClass(subjectClassKey, configKey)); | 348 | + service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), |
| 349 | + configKey); | ||
| 354 | } | 350 | } |
| 355 | 351 | ||
| 356 | } | 352 | } | ... | ... |
-
Please register or login to post a comment