Bug fix/small issues 10122019

CHANGELOG

@@ -1,6 +1,14 @@
 devel
 -----
 
+* Changed HTTP return code for an error case in /_api/cluster/endpoints REST API.
+  Now, if the API is called on a single server, it will return HTTP 501 instead
+  of HTTP 403.
+
+* Changed HTTP return code for an error case in /_api/cluster/agency-dump REST API.
+  Now, if the API is called on a server type other than coordinator, it will return 
+  HTTP 501 instead of HTTP 403.
+
 * Fixed undefined behavior on node delete.
 
 * Fixed agency invalid operation

arangod/Cluster/RestClusterHandler.cpp

@@ -64,14 +64,14 @@ RestStatus RestClusterHandler::execute() {
   }
 
   generateError(
-    Result(TRI_ERROR_FORBIDDEN, "expecting _api/cluster/[endpoints,agency-dump]"));
+    Result(TRI_ERROR_HTTP_NOT_FOUND, "expecting /_api/cluster/[endpoints,agency-dump]"));
 
   return RestStatus::DONE;
 }
 
 void RestClusterHandler::handleAgencyDump() {
   if (!ServerState::instance()->isCoordinator()) {
-    generateError(rest::ResponseCode::FORBIDDEN, TRI_ERROR_FORBIDDEN,
+    generateError(rest::ResponseCode::NOT_IMPLEMENTED, TRI_ERROR_NOT_IMPLEMENTED,
                   "only to be executed on coordinators");
     return;
   }
@@ -108,7 +108,7 @@ void RestClusterHandler::handleCommandEndpoints() {
     ReplicationFeature* replication = ReplicationFeature::INSTANCE;
     if (!replication->isActiveFailoverEnabled() || !AgencyCommManager::isEnabled()) {
       generateError(
-          Result(TRI_ERROR_FORBIDDEN, "automatic failover is not enabled"));
+          Result(TRI_ERROR_NOT_IMPLEMENTED, "automatic failover is not enabled"));
       return;
     }
 
@@ -164,7 +164,7 @@ void RestClusterHandler::handleCommandEndpoints() {
     endpoints.insert(endpoints.begin(), leaderId);
 
   } else {
-    generateError(Result(TRI_ERROR_FORBIDDEN, "cannot serve this request"));
+    generateError(Result(TRI_ERROR_NOT_IMPLEMENTED, "cannot serve this request for this deployment type"));
     return;
   }
 

arangod/GeoIndex/Near.cpp

@@ -181,7 +181,7 @@ std::vector<geo::Interval> NearUtils<CMP>::intervals() {
     // LOG_TOPIC("55f3b", INFO, Logger::FIXME) << "[Scan] 0 to something";
     S2Cap ob = S2Cap(_origin, _outerAngle);
     //_coverer.GetCovering(ob, &cover);
-    if (_scannedCells.empty() == 0) {
+    if (_scannedCells.empty()) {
       _coverer.GetFastCovering(ob, &cover);
     } else {
       std::vector<S2CellId> tmpCover;
@@ -193,7 +193,7 @@ std::vector<geo::Interval> NearUtils<CMP>::intervals() {
   } else if (_innerAngle > _minAngle) {
     // create a search ring
 
-    if (_scannedCells.size() > 0) {
+    if (!_scannedCells.empty()) {
       S2Cap ob(_origin, _outerAngle);  // outer ring
       std::vector<S2CellId> tmpCover;
       _coverer.GetCovering(ob, &tmpCover);
@@ -204,6 +204,7 @@ std::vector<geo::Interval> NearUtils<CMP>::intervals() {
     } else {
       // expensive exact cover
       std::vector<std::unique_ptr<S2Region>> regions;
+      regions.reserve(2);
       S2Cap ib(_origin, _innerAngle);  // inner ring
       regions.push_back(std::make_unique<S2Cap>(ib.Complement()));
       regions.push_back(std::make_unique<S2Cap>(_origin, _outerAngle));

arangod/GeoIndex/Near.h

@@ -67,7 +67,7 @@ struct DocumentsDescending {
 
 /// @brief Helper class to build a simple near query iterator.
 /// Will return points sorted by distance to the target point, can
-/// also filter contains / intersect in regions (on rsesult points and
+/// also filter contains / intersect in regions (on result points and
 /// search intervals). Should be storage engine agnostic
 template <typename CMP = DocumentsAscending>
 class NearUtils {

arangod/RestServer/InitDatabaseFeature.cpp

@@ -119,7 +119,7 @@ void InitDatabaseFeature::prepare() {
   if (!_seenPassword) {
     while (true) {
       std::string password1 =
-          readPassword("Please enter password for root user");
+          readPassword("Please enter a new password for the ArangoDB root user");
 
       if (!password1.empty()) {
         std::string password2 = readPassword("Repeat password");

lib/Geo/GeoJson.cpp

@@ -27,6 +27,7 @@
 #include <vector>
 
 #include <velocypack/Iterator.h>
+#include <velocypack/StringRef.h>
 #include <velocypack/velocypack-aliases.h>
 
 #include <s2/s2loop.h>
@@ -45,23 +46,23 @@
 
 namespace {
 // Have one of these values:
-const std::string kTypeStringPoint = "Point";
-const std::string kTypeStringLineString = "LineString";
-const std::string kTypeStringPolygon = "Polygon";
-const std::string kTypeStringMultiPoint = "MultiPoint";
-const std::string kTypeStringMultiLineString = "MultiLineString";
-const std::string kTypeStringMultiPolygon = "MultiPolygon";
-const std::string kTypeStringGeometryCollection = "GeometryCollection";
+std::string const kTypeStringPoint = "Point";
+std::string const kTypeStringLineString = "LineString";
+std::string const kTypeStringPolygon = "Polygon";
+std::string const kTypeStringMultiPoint = "MultiPoint";
+std::string const kTypeStringMultiLineString = "MultiLineString";
+std::string const kTypeStringMultiPolygon = "MultiPolygon";
+std::string const kTypeStringGeometryCollection = "GeometryCollection";
 }  // namespace
 
 namespace {
-inline bool sameCharIgnoringCase(char a, char b) {
+inline bool sameCharIgnoringCase(char a, char b) noexcept {
   return arangodb::basics::StringUtils::toupper(a) == arangodb::basics::StringUtils::toupper(b);
 }
 }  // namespace
 
 namespace {
-bool sameIgnoringCase(std::string const& s1, std::string const& s2) {
+bool sameIgnoringCase(arangodb::velocypack::StringRef const& s1, std::string const& s2) {
   return s1.size() == s2.size() &&
          std::equal(s1.begin(), s1.end(), s2.begin(), ::sameCharIgnoringCase);
 }
@@ -97,14 +98,17 @@ arangodb::Result parsePoints(VPackSlice const& vpack, bool geoJson,
   vertices.clear();
   VPackSlice coordinates = vpack;
   if (vpack.isObject()) {
-    coordinates = vpack.get("coordinates");
+    coordinates = vpack.get(arangodb::geo::geojson::Fields::kCoordinates);
   }
 
   if (!coordinates.isArray()) {
     return {TRI_ERROR_BAD_PARAMETER, "Coordinates missing"};
   }
 
-  for (VPackSlice pt : VPackArrayIterator(coordinates)) {
+  VPackArrayIterator it(coordinates);
+  vertices.reserve(it.size());
+
+  for (VPackSlice pt : it) {
     if (!pt.isArray() || pt.length() != 2) {
       return {TRI_ERROR_BAD_PARAMETER, "Bad coordinate " + pt.toJson()};
     }
@@ -114,7 +118,7 @@ arangodb::Result parsePoints(VPackSlice const& vpack, bool geoJson,
     if (!lat.isNumber() || !lon.isNumber()) {
       return {TRI_ERROR_BAD_PARAMETER, "Bad coordinate " + pt.toJson()};
     }
-    vertices.push_back(
+    vertices.emplace_back(
         S2LatLng::FromDegrees(lat.getNumber<double>(), lon.getNumber<double>()).ToPoint());
   }
   return {TRI_ERROR_NO_ERROR};
@@ -136,7 +140,7 @@ Type type(VPackSlice const& vpack) {
     return Type::UNKNOWN;
   }
 
-  std::string typeStr = type.copyString();
+  arangodb::velocypack::StringRef typeStr = type.stringRef();
   if (::sameIgnoringCase(typeStr, ::kTypeStringPoint)) {
     return Type::POINT;
   } else if (::sameIgnoringCase(typeStr, ::kTypeStringLineString)) {
@@ -156,55 +160,54 @@ Type type(VPackSlice const& vpack) {
 };
 
 Result parseRegion(VPackSlice const& vpack, ShapeContainer& region) {
-  Result badParam(TRI_ERROR_BAD_PARAMETER, "Invalid GeoJSON Geometry Object.");
-  if (!vpack.isObject()) {
-    return badParam;
-  }
-
-  Type t = type(vpack);
-  switch (t) {
-    case Type::POINT: {
-      S2LatLng ll;
-      Result res = parsePoint(vpack, ll);
-      if (res.ok()) {
-        region.reset(new S2PointRegion(ll.ToPoint()), ShapeContainer::Type::S2_POINT);
+  if (vpack.isObject()) {
+    Type t = type(vpack);
+    switch (t) {
+      case Type::POINT: {
+        S2LatLng ll;
+        Result res = parsePoint(vpack, ll);
+        if (res.ok()) {
+          region.reset(new S2PointRegion(ll.ToPoint()), ShapeContainer::Type::S2_POINT);
+        }
+        return res;
       }
-      return res;
-    }
-    case Type::MULTI_POINT: {
-      return parseMultiPoint(vpack, region);
-    }
-    case Type::LINESTRING: {
-      auto line = std::make_unique<S2Polyline>();
-      Result res = parseLinestring(vpack, *line.get());
-      if (res.ok()) {
-        region.reset(std::move(line), ShapeContainer::Type::S2_POLYLINE);
+      case Type::MULTI_POINT: {
+        return parseMultiPoint(vpack, region);
       }
-      return res;
-    }
-    case Type::MULTI_LINESTRING: {
-      std::vector<S2Polyline> polylines;
-      Result res = parseMultiLinestring(vpack, polylines);
-      if (res.ok()) {
-        region.reset(new S2MultiPolyline(std::move(polylines)),
-                     ShapeContainer::Type::S2_MULTIPOLYLINE);
+      case Type::LINESTRING: {
+        auto line = std::make_unique<S2Polyline>();
+        Result res = parseLinestring(vpack, *line.get());
+        if (res.ok()) {
+          region.reset(std::move(line), ShapeContainer::Type::S2_POLYLINE);
+        }
+        return res;
+      }
+      case Type::MULTI_LINESTRING: {
+        std::vector<S2Polyline> polylines;
+        Result res = parseMultiLinestring(vpack, polylines);
+        if (res.ok()) {
+          region.reset(new S2MultiPolyline(std::move(polylines)),
+                       ShapeContainer::Type::S2_MULTIPOLYLINE);
+        }
+        return res;
+      }
+      case Type::POLYGON: {
+        return parsePolygon(vpack, region);
+      }
+      case Type::MULTI_POLYGON: {
+        return parseMultiPolygon(vpack, region);
+      }
+      case Type::GEOMETRY_COLLECTION: {
+        return Result(TRI_ERROR_NOT_IMPLEMENTED, "GeoJSON type is not supported");
+      }
+      case Type::UNKNOWN: {
+        // will return TRI_ERROR_BAD_PARAMETER
+        break;
       }
-      return res;
-    }
-    case Type::POLYGON: {
-      return parsePolygon(vpack, region);
-    }
-    case Type::MULTI_POLYGON: {
-      return parseMultiPolygon(vpack, region);
-    }
-    case Type::GEOMETRY_COLLECTION:
-      return Result(TRI_ERROR_NOT_IMPLEMENTED, "GeoJSON type is not supported");
-    case Type::UNKNOWN: {
-      return badParam;
     }
   }
 
-  return badParam;
+  return Result(TRI_ERROR_BAD_PARAMETER, "Invalid GeoJSON Geometry Object.");
 }
 
 /// @brief create s2 latlng
@@ -227,9 +230,8 @@ Result parsePoint(VPackSlice const& vpack, S2LatLng& latLng) {
 
 /// https://tools.ietf.org/html/rfc7946#section-3.1.3
 Result parseMultiPoint(VPackSlice const& vpack, ShapeContainer& region) {
-  Result badParam(TRI_ERROR_BAD_PARAMETER, "Invalid GeoJSON Geometry Object.");
   if (!vpack.isObject()) {
-    return badParam;
+    return Result(TRI_ERROR_BAD_PARAMETER, "Invalid GeoJSON Geometry Object.");
   }
 
   if (Type::MULTI_POINT != type(vpack)) {
@@ -269,8 +271,7 @@ Result parsePolygon(VPackSlice const& vpack, ShapeContainer& region) {
   if (!coordinates.isArray()) {
     return Result(TRI_ERROR_BAD_PARAMETER, "coordinates missing");
   }
-  size_t n = coordinates.length();
-
+
   // Coordinates of a Polygon are an array of LinearRing coordinate arrays.
   // The first element in the array represents the exterior ring. Any subsequent
   // elements
@@ -283,8 +284,13 @@ Result parsePolygon(VPackSlice const& vpack, ShapeContainer& region) {
   // - A linear ring MUST follow the right-hand rule with respect to the
   //   area it bounds, i.e., exterior rings are counterclockwise (CCW), and
   //   holes are clockwise (CW).
+  VPackArrayIterator it(coordinates);
+  size_t const n = it.size();
+
   std::vector<std::unique_ptr<S2Loop>> loops;
-  for (VPackSlice loopVertices : VPackArrayIterator(coordinates)) {
+  loops.reserve(n);
+
+  for (VPackSlice loopVertices : it) {
     std::vector<S2Point> vtx;
     Result res = ::parsePoints(loopVertices, /*geoJson*/ true, vtx);
     if (res.fail()) {
@@ -329,7 +335,7 @@ Result parsePolygon(VPackSlice const& vpack, ShapeContainer& region) {
         }
       }
     }
-    loops.push_back(std::make_unique<S2Loop>(vtx, S2Debug::DISABLE));
+    loops.push_back(std::make_unique<S2Loop>(std::move(vtx), S2Debug::DISABLE));
 
     S2Error error;
     if (loops.back()->FindValidationError(&error)) {