Fix requests with empty multiget lists (#204)

When requesting a multiget with an empty list, the generated url was invalid, and caused no requests to be returned.

This fix removes empty parameters, and also short-circuits forward relations when there are no ids available.

naptime-graphql/src/main/scala/org/coursera/naptime/ari/graphql/schema/NaptimePaginatedResourceField.scala

@@ -36,6 +36,7 @@ import sangria.schema.Field
 import sangria.schema.ListType
 import sangria.schema.ObjectType
 import sangria.schema.OptionType
+import sangria.schema.Value
 
 object NaptimePaginatedResourceField extends StrictLogging {
 
@@ -107,7 +108,7 @@ object NaptimePaginatedResourceField extends StrictLogging {
 
             val hasIds = fieldRelationOpt.exists(_.relationType == RelationType.MULTI_GET) ||
               fieldRelationOpt.exists(_.relationType == RelationType.GET)
-            val (updatedArgs, paginationOverride) = if (hasIds) {
+            val (updatedArgs, paginationOverride, isEmpty) = if (hasIds) {
               val startOption = context.arg(NaptimePaginationField.startArgument)
               val limit = context.arg(NaptimePaginationField.limitArgument)
               val ids = args.find(_._1 == "ids").map(_._2).collect {
@@ -129,23 +130,32 @@ object NaptimePaginatedResourceField extends StrictLogging {
 
               val paginationResponse = ResponsePagination(next, Some(ids.size.toLong))
 
-              (args.filterNot(_._1 == "ids") + ("ids" -> paginatedIds), Some(paginationResponse))
+              if (paginatedIds.value.isEmpty || Utilities.jsValueIsEmpty(paginatedIds)) {
+                (args, Some(paginationResponse), true)
+              } else {
+                (args.filterNot(_._1 == "ids") + ("ids" -> paginatedIds), Some(paginationResponse), false)
+              }
             } else {
-              (args, None)
+              (args, None, false)
             }
 
-            DeferredValue(DeferredNaptimeRequest(
-              resourceName, updatedArgs, resourceMergedType, paginationOverride))
-              .map {
-                case Left(error) =>
-                  // TODO(bryan): Replace this with an exception once clients can handle it
-                  NaptimeResponse(List.empty, None, error.url)
-                case Right(response) => {
-                  val limit = context.arg(NaptimePaginationField.limitArgument)
-                  // If more results are returned than requested, only take up to the limit
-                  response.copy(elements = response.elements.take(limit))
-                }
-              }(context.ctx.executionContext)
+            if (isEmpty) {
+              Value(NaptimeResponse(List.empty, None, "No ids found."))
+            } else {
+              DeferredValue(
+                DeferredNaptimeRequest(
+                  resourceName, updatedArgs, resourceMergedType, paginationOverride))
+                .map {
+                  case Left(error) =>
+                    // TODO(bryan): Replace this with an exception once clients can handle it
+                    NaptimeResponse(List.empty, None, error.url)
+                  case Right(response) => {
+                    val limit = context.arg(NaptimePaginationField.limitArgument)
+                    // If more results are returned than requested, only take up to the limit
+                    response.copy(elements = response.elements.take(limit))
+                  }
+                }(context.ctx.executionContext)
+            }
           },
           complexity = Some(
             (ctx, args, childScore) => {

naptime/src/main/scala/org/coursera/naptime/ari/engine/Utilities.scala

@@ -296,6 +296,23 @@ object Utilities extends StrictLogging {
     }
   }
 
+  def jsValueIsEmpty(value: JsValue): Boolean = {
+    value match {
+      case JsArray(arrayElements) =>
+        arrayElements.isEmpty || arrayElements.forall(jsValueIsEmpty)
+      case stringValue: JsString =>
+        stringValue.value.isEmpty
+      case _: JsNumber =>
+        false
+      case _: JsBoolean =>
+        false
+      case _: JsObject =>
+        false
+      case JsNull =>
+        true
+    }
+  }
+
   /**
     * Merges all sub-selections of a request into a single selection
     * @param fields list of selections to merge

version.sbt

@@ -1 +1 @@
-version in ThisBuild := "0.7.0"
+version in ThisBuild := "0.7.1"