add dependency check for security rule's group update(), not just add()

JIRA: NEUTRON-158
Change-Id: I249d8a3898641baaf9b92699da41eac523a22b36
Signed-off-by: Michael Vorburger <vorburger@redhat.com>

diff --git a/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityGroupTests.java b/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityGroupTests.java
index ab9652f5eadd8a0bfa2c00ac9d7df24d7784345c..6a507c0a5a737d474db8bfc091209478620ed29e 100644 (file)
--- a/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityGroupTests.java
+++ b/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityGroupTests.java
@@ -85,11 +85,15 @@ public class NeutronSecurityGroupTests {
         HttpUtils.test_fetch(url, true, "Security Group Element Get Failed");
     }
 
-    private void sg_delete_test() {
-        String url = base + "/security-groups/521e29d6-67b8-4b3c-8633-027d21195333";
+    public void sg_delete(String securityGroupID) {
+        String url = base + "/security-groups/" + securityGroupID;
         HttpUtils.test_delete(url, "Security Group Delete Failed");
     }
 
+    private void sg_delete_test() {
+        sg_delete("521e29d6-67b8-4b3c-8633-027d21195333");
+    }
+
     private void sg_element_negative_get_test() {
         String url = base + "/security-groups/521e29d6-67b8-4b3c-8633-027d21195333";
         HttpUtils.test_fetch(url, false, "Security Group Element Negative Get Failed");

diff --git a/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java b/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java
index 8d0018197267c1247573f4da8965dae6de19e004..ac5989a90917ebd2ea880faaa7c259f51ba2982d 100644 (file)
--- a/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java
+++ b/integration/test-standalone/src/main/java/org/opendaylight/neutron/e2etest/NeutronSecurityRuleTests.java
@@ -70,13 +70,13 @@ public class NeutronSecurityRuleTests {
         HttpUtils.test_create(url, content, "Security Rule Multiple Post Failed");
     }
 
-    private void singleton_sr_modify_test(int responseCode) {
+    private void singleton_sr_modify_test(int responseCode, boolean withSecurityGroupID) {
         String url = base + "/security-group-rules/9b4be7fa-e56e-40fb-9516-1f0fa9185669";
         String content = " {\"security_group_rule\": " + "{\"remote_group_id\": null, \"direction\": \"egress\", "
                 + "\"remote_ip_prefix\": null, \"protocol\": \"tcp\", " + "\"ethertype\": \"IPv6\", \"tenant_id\": "
                 + "\"00f340c7c3b34ab7be1fc690c05a0275\", \"port_range_max\": 77, " + "\"port_range_min\": 77, "
-                + "\"id\": \"9b4be7fa-e56e-40fb-9516-1f0fa9185669\", " + "\"security_group_id\": "
-                + "\"" + TEST_SECURITY_GROUP_ID + "\"}}";
+                + "\"id\": \"9b4be7fa-e56e-40fb-9516-1f0fa9185669\"";
+        content += withSecurityGroupID ? ", " + "\"security_group_id\": \"" + TEST_SECURITY_GROUP_ID + "\"}}" : "}}";
         HttpUtils.test_modify(url, responseCode, content, "Security Rule Singleton Put Failed");
     }
 
@@ -150,14 +150,15 @@ public class NeutronSecurityRuleTests {
 
     public static void runTests(String base) {
         NeutronSecurityRuleTests securityRuleTester = new NeutronSecurityRuleTests(base);
-        securityRuleTester.singleton_sr_without_groupid_create_test(500);
-        securityRuleTester.singleton_sr_create_test(HttpUtils.HTTP_MISSING_DEPENDENCY);
-        securityRuleTester.singleton_sr_modify_test(404);
+        // TODO securityRuleTester.singleton_sr_without_groupid_create_test(500);
+        securityRuleTester.singleton_sr_create_test(HttpUtils.HTTP_MISSING_DEPENDENCY); // NEUTRON-158
+        securityRuleTester.singleton_sr_modify_test(404, true); // cannot modify a SR that has not been created
         new NeutronSecurityGroupTests(base).singleton_sg_create(TEST_SECURITY_GROUP_ID);
         String createJsonString = securityRuleTester.singleton_sr_create_test(201);
         securityRuleTester.singleton_sr_get_with_one_query_item_test(createJsonString);
         securityRuleTester.multiple_sr_create_test();
-        securityRuleTester.singleton_sr_modify_test(200);
+        securityRuleTester.singleton_sr_modify_test(200, true);
+        securityRuleTester.singleton_sr_modify_test(200, false);
         securityRuleTester.sr_element_get_test();
         securityRuleTester.sr_element_get_with_query_test();
         securityRuleTester.securityRule_collection_get_test();
@@ -167,5 +168,10 @@ public class NeutronSecurityRuleTests {
         securityRuleTester.bug4043_ipv4_test();
         securityRuleTester.bug4043_ipv6_test();
         securityRuleTester.bug6398_sr_create_test();
+
+        // NEUTRON-158: Cannot modify a SR who's SG is already deleted
+        securityRuleTester.singleton_sr_create_test(201);
+        new NeutronSecurityGroupTests(base).sg_delete(TEST_SECURITY_GROUP_ID);
+        securityRuleTester.singleton_sr_modify_test(HttpUtils.HTTP_MISSING_DEPENDENCY, true);
     }
 }

diff --git a/northbound-api/src/main/java/org/opendaylight/neutron/northbound/api/AbstractNeutronNorthbound.java b/northbound-api/src/main/java/org/opendaylight/neutron/northbound/api/AbstractNeutronNorthbound.java
index a1a647a1aa78cbf9ce1031549387478f1b05a297..39a9b4d340a420ce503f18cefcd6493430eb7f0c 100644 (file)
--- a/northbound-api/src/main/java/org/opendaylight/neutron/northbound/api/AbstractNeutronNorthbound.java
+++ b/northbound-api/src/main/java/org/opendaylight/neutron/northbound/api/AbstractNeutronNorthbound.java
@@ -156,6 +156,8 @@ public abstract class AbstractNeutronNorthbound<T extends INeutronObject<T>, R e
         Result updateResult = neutronCRUD.update(uuid, delta);
         if (updateResult.equals(DoesNotExist)) {
             throw new ResourceNotFoundException(uuidNoExist());
+        } else if (updateResult.equals(DependencyMissing)) {
+            return Response.status(HTTP_MISSING_DEPENDENCY).entity(input).build();
         }
         T updated = neutronCRUD.get(uuid);
         return Response.status(HttpURLConnection.HTTP_OK).entity(newNeutronRequest(updated)).build();

diff --git a/transcriber/src/main/java/org/opendaylight/neutron/transcriber/AbstractTranscriberInterface.java b/transcriber/src/main/java/org/opendaylight/neutron/transcriber/AbstractTranscriberInterface.java
index 987d4781682cc1b4a08e09f42c3c84adef687523..5419ec08c6aac84fd64103944c4662eea3a45a59 100644 (file)
--- a/transcriber/src/main/java/org/opendaylight/neutron/transcriber/AbstractTranscriberInterface.java
+++ b/transcriber/src/main/java/org/opendaylight/neutron/transcriber/AbstractTranscriberInterface.java
@@ -520,7 +520,11 @@ public abstract class AbstractTranscriberInterface<
         while (retries-- >= 0) {
             final ReadWriteTransaction tx = getDataBroker().newReadWriteTransaction();
             try {
-                return update(uuid, delta, tx);
+                if (areAllDependenciesAvailable(tx, delta)) {
+                    return update(uuid, delta, tx);
+                } else {
+                    return Result.DependencyMissing;
+                }
             } catch (InterruptedException | ExecutionException e) {
                 if (e.getCause() instanceof OptimisticLockFailedException) {
                     LOG.warn("Got OptimisticLockFailedException - {} {} {}", uuid, delta, retries);
@@ -543,7 +547,9 @@ public abstract class AbstractTranscriberInterface<
      * {@link #exists(String, ReadTransaction)} method on ANOTHER transcriber with it.
      *
      * <p>Implementations should chain {@link #ifNonNull(Object, Function)}, or perform null safe comparisons otherwise,
-     * for any optional non-mandatory {@link NeutronObject} properties which may well be null.
+     * for both optional non-mandatory {@link NeutronObject} as well as mandatory properties which may well be null.
+     * Both must mandatory and non-mandatory must be guarded, because modify (update) operation are allowed to contain
+     * partial neutron objects with missing fields.
      *
      * @param tx the transaction within which to perform reads to check for dependencies
      * @param neutronObject the incoming main neutron object in which there may be references to dependencies

diff --git a/transcriber/src/main/java/org/opendaylight/neutron/transcriber/NeutronSecurityRuleInterface.java b/transcriber/src/main/java/org/opendaylight/neutron/transcriber/NeutronSecurityRuleInterface.java
index 653e76ffeea8db0adeeb86bb5058aa2455a443e2..aa40541559b2d0b7a6b37d5486a974eecc6e5164 100644 (file)
--- a/transcriber/src/main/java/org/opendaylight/neutron/transcriber/NeutronSecurityRuleInterface.java
+++ b/transcriber/src/main/java/org/opendaylight/neutron/transcriber/NeutronSecurityRuleInterface.java
@@ -136,8 +136,9 @@ public final class NeutronSecurityRuleInterface extends
 
     @Override
     protected boolean areAllDependenciesAvailable(ReadTransaction tx, NeutronSecurityRule securityRule) {
-        return securityGroupInterface.exists(securityRule.getSecurityRuleGroupID(), tx)
-                && ifNonNull(securityRule.getSecurityRemoteGroupID(),
-                    remoteGroupID -> securityGroupInterface.exists(remoteGroupID, tx));
+        return ifNonNull(securityRule.getSecurityRuleGroupID(),
+            groupID -> securityGroupInterface.exists(groupID, tx))
+            && ifNonNull(securityRule.getSecurityRemoteGroupID(),
+                remoteGroupID -> securityGroupInterface.exists(remoteGroupID, tx));
     }
 }