[GitHub] markusthoemmes commented on a change in pull request #2592: Make the result of response consistent for both Wsk CLI and REST

[gi...@git.apache.org]

markusthoemmes commented on a change in pull request #2592: Make the result of response consistent for both Wsk CLI and REST
URL: https://github.com/apache/incubator-openwhisk/pull/2592#discussion_r133750881
 
 

 ##########
 File path: tests/src/test/scala/common/rest/WskRest.scala
 ##########
 @@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package common.rest
+
+import scala.util.Try
+
+import akka.http.scaladsl.model.StatusCode
+import akka.http.scaladsl.model.StatusCodes.BadRequest
+import spray.json.JsObject
+import spray.json.JsValue
+import spray.json.pimpString
+import spray.json.DefaultJsonProtocol._
+
+import common.TestUtils.RunResult
+
+class WskRest() {
+}
+
+object RestResult {
+
+    val codeConversion = 256
+
+    def getField(obj: JsObject, key: String): String = {
+        val list = obj.getFields(key)
+        if (list.size == 0) {
+            ""
+        }
+        else {
+            Try {
+                list(0).convertTo[String]
+            } getOrElse
+                list(0).convertTo[Int].toString
+        }
+    }
+
+    def getFieldJsObject(obj: JsObject, key: String): JsObject = {
+        val list = obj.getFields(key)
+        if (list.size == 0) JsObject() else list(0).toJson.asJsObject
+    }
+
+    def getFieldJsValue(obj: JsObject, key: String): JsValue = {
+        val list = obj.getFields(key)
+        if (list.size == 0) JsObject() else list(0).toJson
+    }
+
+    def getFieldListJsObject(obj: JsObject, key: String): Vector[JsObject] = {
+        val list = obj.getFields(key)
+        if (list.size == 0) Vector(JsObject()) else list(0).toJson.convertTo[Vector[JsObject]]
+    }
 
 Review comment:
   All of those helpers look a lot like old-school javaish way to write this, which can be fine but it's overly verbose and I think entirely unneeded in this case.
   
   Is there a specific reason all of those have empty response like "fallbacks"? Could you give an example where that's necessary?
   
   Some proposed equivalents working with scala idioms like `Option`:
   
   ```scala
   obj.fields.get(key).map(_.compactPrint).getOrElse("") == getField(obj, key)
   obj.fields.get(key).map(_.asJsObject).getOrElse(JsObject()) == getFieldJsObject(obj, key)
   obj.fields.get(key).getOrElse(JsObject()) == getFieldJsValue(obj, key)
   ```
   
   You can see, that those methods do compose quite nicely and I am not opposed to helper methods at all in general. In this case though, they seem a bit off.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services