Bug fix 3.5/arangosh gharial vertex deletion (#9926)

* typo in route

* unused function removed

* added general graph standalone and cluster tests, formatting

* removed fail, removed print

* changelog

* Update CHANGELOG

CHANGELOG

@@ -52,6 +52,10 @@ v3.5.1 (XXXX-XX-XX)
   This change also makes arangorestore restore the `_system` database last if it
   is started with the `--all-databases` option.
 
+* Fixed the removal (including a collection drop) of an orphanCollection from a
+  graph definition when using the arango shell. The boolean flag whether to drop
+  the collection or not was not transferred properly.
+
 * Check for duplicate server endpoints registered in the agency in sub-keys of
   `/Current/ServersRegistered`.
 

js/client/modules/@arangodb/general-graph.js

@@ -119,9 +119,9 @@ CommonGraph.prototype._addVertexCollection = function (name, createCollection) {
 CommonGraph.prototype._removeVertexCollection = function (name, dropCollection) {
   let uri = GRAPH_PREFIX + encodeURIComponent(this.__name) + "/vertex/" + encodeURIComponent(name);
   if (dropCollection === true) {
-    uri += "?dropCollections=true";
+    uri += "?dropCollection=true";
   } else {
-    uri += "?dropCollections=false";
+    uri += "?dropCollection=false";
   }
   const requestResult = arangosh.checkRequestResult(db._connection.DELETE(uri));
   const graph = requestResult.graph;

js/common/modules/@arangodb/general-graph-common.js

@@ -1566,44 +1566,6 @@ class Graph {
     return this.__orphanCollections;
   }
 
-// //////////////////////////////////////////////////////////////////////////////
-// / @brief was docuBlock JSF_general_graph__removeVertexCollection
-// //////////////////////////////////////////////////////////////////////////////
-
-  _removeVertexCollection (vertexCollectionName, dropCollection) {
-    var err;
-    if (db._collection(vertexCollectionName) === null) {
-      err = new ArangoError();
-      err.errorNum = arangodb.errors.ERROR_GRAPH_VERTEX_COL_DOES_NOT_EXIST.code;
-      err.errorMessage = arangodb.errors.ERROR_GRAPH_VERTEX_COL_DOES_NOT_EXIST.message;
-      throw err;
-    }
-
-    var index = this.__orphanCollections.indexOf(vertexCollectionName);
-    if (index === -1) {
-      err = new ArangoError();
-      err.errorNum = arangodb.errors.ERROR_GRAPH_NOT_IN_ORPHAN_COLLECTION.code;
-      err.errorMessage = arangodb.errors.ERROR_GRAPH_NOT_IN_ORPHAN_COLLECTION.message;
-      throw err;
-    }
-
-    if (dropCollection) {
-      checkRWPermission(vertexCollectionName);
-    }
-
-    this.__orphanCollections.splice(index, 1);
-    delete this[vertexCollectionName];
-    db._graphs.update(this.__name, {orphanCollections: this.__orphanCollections});
-
-    if (dropCollection === true) {
-      var graphs = exports._listObjects();
-      if (checkIfMayBeDropped(vertexCollectionName, null, graphs)) {
-        db._drop(vertexCollectionName);
-      }
-    }
-    updateBindCollections(this);
-  }
-
 // //////////////////////////////////////////////////////////////////////////////
 // / @brief was docuBlock JSF_general_graph_connectingEdges
 // //////////////////////////////////////////////////////////////////////////////

tests/js/common/shell/general-graph-creation-cluster-spec.js

@@ -31,6 +31,7 @@
 const arangodb = require('@arangodb');
 const expect = require('chai').expect;
 const graph = require("@arangodb/general-graph");
+const _ = require("lodash");
 const db = arangodb.db;
 
 describe('General graph creation', function () {
@@ -357,31 +358,42 @@ describe('General graph creation', function () {
         // We modify the first relation by adding a new vertex collection
         let rel = graph._relation(en, vn, vn3);
         g._editEdgeDefinitions(rel);
-
         expect(db._collection(vn3)).to.exist;
       });
 
       describe('replication factor', function () {
-
         it(`should be ${replicationFactor} for vertex collection`, function () {
           let props = db._collection(vn3).properties();
           expect(props.replicationFactor).to.equal(replicationFactor);
         });
-
       });
 
       describe('number of shards', function () {
-
         it(`should be ${numberOfShards} for vertex collection`, function () {
           let props = db._collection(vn3).properties();
           expect(props.numberOfShards).to.equal(numberOfShards);
         });
+      });
+    });
 
+    describe('modify vertices', function () {
+      it(`remove a vertex collection from the graph definition and also drop the collection`, function () {
+        expect(db[on].name() === on);
+        g._removeVertexCollection(on, true);
+
+        // check that the collection is really dropped
+        // using collections list
+        var collections = db._collections();
+        var found = false;
+        _.each(collections, function (collection) {
+          if (collection.name() === on) {
+            found = true;
+          }
+        });
+
+        expect(found).to.be.false;
+      });
     });
 
   });
-
-
-  });
-
 });

tests/js/common/shell/shell-general-graph.js

@@ -326,8 +326,7 @@ function GeneralGraphCreationSuite() {
       try {
         graph._relation("", [vn1, vn2], [vn1, vn2]);
         fail();
-      }
+      } catch (err) {
-      catch (err) {
         assertEqual(err.errorMessage, "Invalid parameter type. arg1 must be non empty string");
       }
     },
@@ -336,8 +335,7 @@ function GeneralGraphCreationSuite() {
       try {
         graph._relation([vn1, vn2]);
         fail();
-      }
+      } catch (err) {
-      catch (err) {
         assertEqual(err.errorMessage, "Invalid number of arguments. Expected: 3");
       }
     },
@@ -371,8 +369,7 @@ function GeneralGraphCreationSuite() {
         graph._relation("",
           [vn1, vn2], [vn3, vn4]);
         fail();
-      }
+      } catch (err) {
-      catch (err) {
         assertEqual(err.errorMessage, "Invalid parameter type. arg1 must be non empty string");
       }
     },
@@ -381,8 +378,7 @@ function GeneralGraphCreationSuite() {
       try {
         graph._relation([vn1, vn2], [vn3, vn4]);
         fail();
-      }
+      } catch (err) {
-      catch (err) {
         assertEqual(err.errorMessage, "Invalid number of arguments. Expected: 3");
       }
     },
@@ -393,8 +389,7 @@ function GeneralGraphCreationSuite() {
         param[vn1] = vn2;
         graph._relation(rn, param, vn3);
         fail();
-      }
+      } catch (err) {
-      catch (err) {
         assertEqual(err.errorMessage, "Invalid parameter type. arg2 must be non empty string or array");
       }
 
@@ -407,8 +402,7 @@ function GeneralGraphCreationSuite() {
         param[vn1] = vn2;
         graph._relation(rn, vn3, param);
         fail();
-      }
+      } catch (err) {
-      catch (err) {
         assertEqual(err.errorMessage, "Invalid parameter type. arg3 must be non empty string or array");
       }
     },
@@ -957,7 +951,6 @@ function GeneralGraphCreationSuite() {
     },
 
     test_editEdgeDefinitionFromExistingGraph3: function () {
-
       var dr1 = graph._relation(ec1, _.cloneDeep([vc1]), _.cloneDeep([vc1, vc2])),
         dr2 = graph._relation(ec1, _.cloneDeep([vc3]), _.cloneDeep([vc4, vc5])),
         dr3 = graph._relation(ec2, _.cloneDeep([vc2]), _.cloneDeep([vc2, vc3])),
@@ -985,10 +978,28 @@ function GeneralGraphCreationSuite() {
         graph._drop(gN2, true);
       } catch (ignore) {
       }
+    },
 
+    test_createGraphAndDropAVertexCollectionAfterwards: function () {
+      try {
+        var gr = graph._create("gg",
+          [graph._relation("edges", "verts", "verts")],
+          ["other"]
+        );
+        assertTrue(db["other"].name() === "other");
+        gr._removeVertexCollection("other", true);
+        var collections = db._collections();
+        var found = false;
+        _.each(collections, function (collection) {
+          if (collection.name() === "other") {
+            found = true;
           }
-
+        });
-
+        assertFalse(found);
+      } finally {
+        graph._drop("gg", true);
+      }
+    },
   };
 }
 
@@ -1217,8 +1228,14 @@ function EdgesAndVerticesSuite() {
       var myED = "unitTestEdgeCollection4711";
       var myVD1 = "unitTestVertexCollection4711";
       var myVD2 = "unitTestVertexCollection4712";
-      try {graph._drop(myGraphName, true);} catch (ignore){}
+      try {
-      try {db._drop(myED);} catch (ignore){}
+        graph._drop(myGraphName, true);
+      } catch (ignore) {
+      }
+      try {
+        db._drop(myED);
+      } catch (ignore) {
+      }
       try {
         graph._create(
           myGraphName,
@@ -1567,8 +1584,7 @@ function EdgesAndVerticesSuite() {
         try {
           g[ec2].save(v, v, {});
           fail();
-        }
+        } catch (e) {
-        catch (e) {
           assertEqual(
             e.errorMessage,
             arangodb.errors.ERROR_ARANGO_DOCUMENT_HANDLE_BAD.message
@@ -1594,8 +1610,7 @@ function EdgesAndVerticesSuite() {
     test_getFromVertexInvalidHandle: function () {
       try {
         g._fromVertex("foobar");
-      }
+      } catch (e) {
-      catch (e) {
         assertEqual(e.errorNum, ERRORS.ERROR_ARANGO_DOCUMENT_HANDLE_BAD.code);
       }
     },
@@ -1603,8 +1618,7 @@ function EdgesAndVerticesSuite() {
     test_getFromVertexInvalidCollectionType: function () {
       try {
         g._fromVertex(vc1 + "/foobar");
-      }
+      } catch (e) {
-      catch (e) {
         assertEqual(e.errorNum, ERRORS.ERROR_GRAPH_EDGE_COL_DOES_NOT_EXIST.code);
       }
     },
@@ -1612,8 +1626,7 @@ function EdgesAndVerticesSuite() {
     test_getFromVertexNonexistingCollection: function () {
       try {
         g._fromVertex("UnitTestsCollectionDoesNotExist/foobar");
-      }
+      } catch (e) {
-      catch (e) {
         assertEqual(e.errorNum, ERRORS.ERROR_GRAPH_EDGE_COL_DOES_NOT_EXIST.code);
       }
     }
@@ -1950,16 +1963,20 @@ function OrphanCollectionSuite() {
     tearDown: function () {
       try {
         graph._drop(gN1, true);
-      } catch(ignore) { }
+      } catch (ignore) {
+      }
       try {
         graph._drop(gN2, true);
-      } catch(ignore) { }
+      } catch (ignore) {
+      }
       try {
         db[vC1].drop();
-      } catch (ignore) {}
+      } catch (ignore) {
+      }
       try {
         db[vC4].drop();
-      } catch (ignore) {}
+      } catch (ignore) {
+      }
     },
 
     test_getOrphanCollection: function () {
@@ -2365,7 +2382,11 @@ function MeasurementsMovedFromAQLSuite() {
     },
 
     testAbsoluteEccentricityInboundWeight: function () {
-      var actual = g._absoluteEccentricity(vertexIds.Anton, {direction: "inbound", weightAttribute: "entfernung", defaultWeight: 80});
+      var actual = g._absoluteEccentricity(vertexIds.Anton, {
+        direction: "inbound",
+        weightAttribute: "entfernung",
+        defaultWeight: 80
+      });
       var expected = {};
       expected[vertexIds.Anton] = 250.2;
       validateNumericValues(actual, expected);
@@ -2379,7 +2400,11 @@ function MeasurementsMovedFromAQLSuite() {
     },
 
     testAbsoluteEccentricityOutboundWeight: function () {
-      var actual = g._absoluteEccentricity(vertexIds.Gerda, {direction: "outbound", weightAttribute: "entfernung", defaultWeight: 80});
+      var actual = g._absoluteEccentricity(vertexIds.Gerda, {
+        direction: "outbound",
+        weightAttribute: "entfernung",
+        defaultWeight: 80
+      });
       var expected = {};
       expected[vertexIds.Gerda] = 380.2;
 
@@ -2396,7 +2421,11 @@ function MeasurementsMovedFromAQLSuite() {
     },
 
     testAbsoluteEccentricityExampleWeight: function () {
-      var actual = g._absoluteEccentricity({gender: "female"}, {direction: "outbound", weightAttribute: "entfernung", defaultWeight: 80});
+      var actual = g._absoluteEccentricity({gender: "female"}, {
+        direction: "outbound",
+        weightAttribute: "entfernung",
+        defaultWeight: 80
+      });
       var expected = {};
       expected[vertexIds.Berta] = 580.2;
       expected[vertexIds.Gerda] = 380.2;
@@ -2419,7 +2448,11 @@ function MeasurementsMovedFromAQLSuite() {
     },
 
     testAbsoluteEccentricityAllWeight: function () {
-      var actual = g._absoluteEccentricity({}, {direction: "outbound", weightAttribute: "entfernung", defaultWeight: 80});
+      var actual = g._absoluteEccentricity({}, {
+        direction: "outbound",
+        weightAttribute: "entfernung",
+        defaultWeight: 80
+      });
       var expected = {};
       expected[vertexIds.Anton] = 0;
       expected[vertexIds.Berta] = 580.2;