소스 검색

Fixed up unit tests and added test for UnitMeasure guess function.

Fixed some issues from outdataed unit tests.

Added unit test infrastructure to shared library and linked that into top-level test call.
Re-wrote guess function to avoid discovered bad cases and cover discovered good cases.

Fixed issue with optional value in FDC API library.
Thomas Flucke 2 년 전
부모
커밋
b2727116da

+ 13 - 6
build.sbt

@@ -17,10 +17,17 @@ lazy val server: Project = (project in file("server"))
     scalaJSProjects := Seq(fdcJs, sharedJs, client),
     Assets / pipelineStages := Seq(scalaJSPipeline),
     Compile / compile := ((Compile / compile) dependsOn scalaJSPipeline).value,
+
+    Test / test := {
+      (Test / test).value
+      (sharedJvm / Test /test).value
+    },
+    
     Test / javaOptions += "-Dconfig.file=conf/testing.conf",
     libraryDependencies ++= Seq(
       guice,
-      "codes.reactive" %% "scala-time" % "0.4.2",
+      "codes.reactive"   %% "scala-time"    % "0.4.2",
+      "net.ruippeixotog" %% "scala-scraper" % "3.1.0",
       // OAuth
       "com.nulab-inc" %% "scala-oauth2-core" % "1.5.0",
       "com.nulab-inc" %% "play2-oauth2-provider" % "1.5.0",
@@ -35,15 +42,14 @@ lazy val server: Project = (project in file("server"))
       // Mogno + ORM
       "org.mongodb.scala" %% "mongo-scala-driver" % "4.1.0",
       //"com.scalableminds" %% "mongev" % "0.5.0",
-      //"com.scalableminds" %% "mongev"             % "0.5.0",
       // Testing
       specs2 % Test,
       "org.scalatestplus.play" %% "scalatestplus-play" % "5.0.0" % Test
     )
   )
   .enablePlugins(PlayScala)
-  .dependsOn(sharedJvm)
-  .dependsOn(fdcJvm)
+  .dependsOn(sharedJvm % "compile->compile;test->test")
+  .dependsOn(fdcJvm % "compile->compile;test->test")
 
 import com.tflucke.webroutes.endpoints.PlayEndpointFile
 import org.scalajs.linker.interface.ModuleInitializer
@@ -81,13 +87,14 @@ lazy val shared = crossProject(JSPlatform, JVMPlatform)
     Compile / apiDefinitions += PlayEndpointFile(server),
     libraryDependencies += "com.typesafe.play" %%% "play-json" % "2.9.2",
     // Temporary workaround due to mongodb limitations
-    libraryDependencies += "org.julienrf" %%% "play-json-derived-codecs" % "8.0.0"
+    libraryDependencies += "org.julienrf" %%% "play-json-derived-codecs" % "8.0.0",
   )
   .enablePlugins(RestRPC)
   .jsConfigure(_ enablePlugins ScalaJSWeb)
 
 lazy val sharedJvm = shared.jvm.dependsOn(fdcJvm).settings(
-  libraryDependencies += "org.mongodb.scala" %% "mongo-scala-bson" % "4.1.0"
+  libraryDependencies += "org.mongodb.scala" %% "mongo-scala-bson" % "4.1.0",
+  libraryDependencies += "org.scalatest"     %% "scalatest"        % "3.0.8" % Test,
 )
 lazy val sharedJs = shared.js.dependsOn(fdcJs).settings(
   libraryDependencies ++= Seq(

+ 2 - 2
fdc/shared/src/main/scala/gov/usda/nal/fdc/models/AbridgedFoodNutrient.scala

@@ -3,10 +3,10 @@ package gov.usda.nal.fdc.models
 case class AbridgedFoodNutrient(
   val nutrientId: Short,
   val nutrientNumber: String,
-  val unitName: String,
+  val unitName: Option[String],
   val derivationDescription: Option[String] = None,
   val name: Option[String] = None,
-  val value: Float = 0.0f,
+  val value: Option[Float] = None,
   val derivationCode: Option[String] = None
 )
 

+ 8 - 1
server/app/com/weEat/controllers/FoodController.scala

@@ -13,7 +13,6 @@ import scala.util.{Success,Failure}
 import com.weEat.models.Authorization
 import scalaoauth2.provider.{AuthInfoRequest,OAuth2ProviderActionBuilders}
 import com.weEat.services.OAuth2Service
-import org.mongodb.scala.model.Filters._
 
 @Singleton
 class FoodController @Inject()(
@@ -187,4 +186,12 @@ class FoodController @Inject()(
       )
     }
   }
+
+  def getByFdcId(fdcId: Long): Future[Option[USDANodeId]] =
+    withCollection(FoodNodeCollection) { (collection) =>
+      collection.find(equal("fdcId", fdcId))
+        .first()
+        .toFuture()
+        .map((foodNode) => Option(foodNode.asInstanceOf[USDANodeId]))
+    }.flatten
 }

+ 7 - 0
server/app/com/weEat/controllers/UserController.scala

@@ -153,4 +153,11 @@ class UserController @Inject()(
       case None => NotFound("No such user with this id")
     })
   }
+
+  def deleteUser(id: ObjectId) = {
+    withCollection(User) { (collection) =>
+      collection.findOneAndDelete(equal("_id", id))
+        .toFuture()
+      }
+  }.flatten
 }

+ 29 - 8
server/test/IntegrationSpec.scala

@@ -1,29 +1,34 @@
+import com.weEat.controllers.UserController
+import javax.inject.Inject
 import play.api.test.Helpers._
-import play.api.test.{FakeRequest}
-import play.api.libs.json.{JsObject,Json}
+import play.api.test.FakeRequest
+import play.api.libs.json.Json
 import org.openqa.selenium.By
 import org.scalatest.{Assertion,BeforeAndAfterAll,BeforeAndAfterEach}
 import org.scalatestplus.play._
 import org.scalatestplus.play.guice.GuiceOneServerPerSuite
 import com.weEat.shared.models.User
-import scala.concurrent.{ExecutionContext,Future,blocking}
+import scala.concurrent.{Future,blocking}
 import scala.concurrent.duration._
 import scala.language.postfixOps
 import java.util.concurrent.TimeoutException
+import org.bson.types.ObjectId
 
-class IntegrationSpec extends PlaySpec
+class IntegrationSpec @Inject()(
+  userController: UserController
+) extends PlaySpec
     with BeforeAndAfterAll
     with BeforeAndAfterEach
     with GuiceOneServerPerSuite
     with AllBrowsersPerTest {
 
   val users = Seq(
-    (User("test", "user", "tuser@sample.org"), "password"),
-    (User("another", "user", "usert@sample.org"), "password")
+    (User(new ObjectId(), "test", "user", "tuser@sample.org"), "password"),
+    (User(new ObjectId(), "another", "user", "usert@sample.org"), "password")
   )
 
   override def beforeAll() = {
-    val Some(resp) = route(app, FakeRequest(PUT, "/user/").withJsonBody(
+    val Some(resp) = route(app, FakeRequest(PUT, "/v1/user/").withJsonBody(
       Json.parse(s"""|{
                      |  "fname": "${users(0)._1.fname}",
                      |  "lname": "${users(0)._1.lname}",
@@ -34,6 +39,22 @@ class IntegrationSpec extends PlaySpec
     status(resp) mustEqual OK
   }
 
+  override def afterAll() = {
+    implicit val ec: scala.concurrent.ExecutionContext = scala.concurrent.ExecutionContext.global
+    Future.sequence(Seq(
+      userController.deleteUser(users(0)._1._id),
+    )).map { (deletes) =>
+      deletes must matchPattern {
+        case Seq(_) =>
+      }
+    }
+    
+    // val Some(resp) = route(app, FakeRequest(DELETE, s"/v1/user/${users(0)._id}"))
+    // status(resp) mustEqual OK
+    // val Some(resp) = route(app, FakeRequest(DELETE, s"/v1/user/${users(1)._id}"))
+    // status(resp) mustEqual OK
+  }
+
   override lazy val browsers = Vector(
     FirefoxInfo(firefoxProfile),
     //ChromeInfo
@@ -90,7 +111,7 @@ class IntegrationSpec extends PlaySpec
       }
 
       def testUserList() = {
-        val Some(resp) = route(app, FakeRequest(GET, "/user/"))
+        val Some(resp) = route(app, FakeRequest(GET, "/v1/user/"))
         status(resp) mustEqual OK
         contentAsJson(resp).as[Seq[String]] must matchPattern {
           case Seq(user1, user2) =>

+ 43 - 26
server/test/ApplicationSpec.scala → server/test/OAuthSpec.scala

@@ -1,16 +1,20 @@
+import com.weEat.controllers.UserController
 import play.api.test.Helpers._
-import play.api.test.{FakeRequest,WithApplication}
+import play.api.test.FakeRequest
 import play.api.mvc.{Results,Headers}
-import play.api.libs.json.{JsObject,Json}
+import play.api.libs.json.Json
 import org.scalatest.BeforeAndAfterAll
-import org.scalatest.tagobjects.Slow
 import org.scalatestplus.play._
 import org.scalatestplus.play.guice.GuiceOneServerPerSuite
 import com.weEat.shared.models.{User,UserAuthorization}
-import scala.concurrent.duration._
 import java.util.Base64
+import org.bson.types.ObjectId
+import scala.concurrent.Future
+import javax.inject.Inject
 
-class OAuthSpec extends PlaySpec
+class OAuthSpec @Inject()(
+  userController: UserController
+) extends PlaySpec
     with BeforeAndAfterAll
     with Results
     with GuiceOneServerPerSuite {
@@ -21,8 +25,8 @@ class OAuthSpec extends PlaySpec
   val lname = "user"
 
   val users = Seq(
-    (User("test", "user", "tuser@sample.org"), "password"),
-    (User("another", "user", "usert@sample.org"), "password")
+    (User(new ObjectId(), "test", "user", "tuser@sample.org"), "password"),
+    (User(new ObjectId(), "another", "user", "usert@sample.org"), "password")
   )
 
   implicit class CSRFWrapper[T](requ: FakeRequest[T]) {
@@ -38,8 +42,8 @@ class OAuthSpec extends PlaySpec
     Headers(("Authorization" -> s"${auth.tokenType} ${auth.accessToken}"))  
 
   override def beforeAll() = {
-    for ((User(fname, lname, email), password) <- users) {
-      val Some(resp) = route(app, FakeRequest(PUT, "/user/").withJsonBody(
+    for ((User(_, fname, lname, email), password) <- users) {
+      val Some(resp) = route(app, FakeRequest(PUT, "/v1/user/").withJsonBody(
         Json.parse(s"""|{
                        |  "fname": "$fname",
                        |  "lname": "$lname",
@@ -51,6 +55,22 @@ class OAuthSpec extends PlaySpec
     }
   }
 
+  override def afterAll() = {
+    implicit val ec: scala.concurrent.ExecutionContext = scala.concurrent.ExecutionContext.global
+    Future.sequence(Seq(
+      userController.deleteUser(users(0)._1._id),
+    )).map { (deletes) =>
+      deletes must matchPattern {
+        case Seq(_) =>
+      }
+    }
+
+    // val Some(resp) = route(app, FakeRequest(DELETE, s"/v1/user/${users(0)._id}"))
+    // status(resp) mustEqual OK
+    // val Some(resp) = route(app, FakeRequest(DELETE, s"/v1/user/${users(1)._id}"))
+    // status(resp) mustEqual OK
+  }
+
   "the token endpoint" should {
     "reject an empty request" in {
       val Some(resp) = route(app, FakeRequest(POST, "/authorize/")
@@ -98,7 +118,7 @@ class OAuthSpec extends PlaySpec
       )
       status(resp) mustEqual OK
       contentAsJson(resp).as[UserAuthorization] must matchPattern {
-        case UserAuthorization(_, _, _, _) =>
+        case UserAuthorization(_, _, _, _, _) =>
       }
     }
 
@@ -113,10 +133,10 @@ class OAuthSpec extends PlaySpec
       val resp1Auth = contentAsJson(resp1).as[UserAuthorization]
       val resp2Auth = contentAsJson(resp2).as[UserAuthorization]
       resp1Auth must matchPattern {
-        case UserAuthorization(_, _, _, _) =>
+        case UserAuthorization(_, _, _, _, _) =>
       }
       resp2Auth must matchPattern {
-        case UserAuthorization(_, _, _, _) =>
+        case UserAuthorization(_, _, _, _, _) =>
       }
       resp1Auth.accessToken mustNot equal(resp2Auth.accessToken)
       resp1Auth.refreshToken mustNot equal(resp2Auth.refreshToken)
@@ -194,7 +214,7 @@ class OAuthSpec extends PlaySpec
       status(resp) mustEqual OK
       val newAuth = contentAsJson(resp).as[UserAuthorization]
       newAuth must matchPattern {
-        case UserAuthorization(_, _, _, _) =>
+        case UserAuthorization(_, _, _, _, _) =>
       }
       newAuth.accessToken mustNot equal(auth.accessToken)
       newAuth.refreshToken mustNot equal(auth.refreshToken)
@@ -217,7 +237,7 @@ class OAuthSpec extends PlaySpec
       )
       status(resp1) mustEqual OK
       contentAsJson(resp1).as[UserAuthorization] must matchPattern {
-        case UserAuthorization(_, _, _, _) =>
+        case UserAuthorization(_, _, _, _, _) =>
       }
       val Some(resp2) = route(app, FakeRequest(POST, "/authorize/")
         .withJsonBody(Json.parse(s"""|{
@@ -293,8 +313,7 @@ class OAuthSpec extends PlaySpec
         .withHeaders(makeAuthHeader(users(0)._1.email, users(0)._2))
         .withCSRFToken()
       ).get).as[UserAuthorization]
-      val fakeToken = (auth.refreshToken.charAt(0)+1) +
-        auth.refreshToken.substring(1);
+      val fakeToken = s"${auth.refreshToken.charAt(0)+1}${auth.refreshToken.substring(1)}";
       val Some(resp) = route(app, FakeRequest(POST, "/authorize/")
         .withJsonBody(Json.parse(s"""|{
                                      |  "grant_type": "refresh_token",
@@ -318,7 +337,7 @@ class OAuthSpec extends PlaySpec
         .withHeaders(makeAuthHeader(email, password))
         .withCSRFToken()
       ).get).as[UserAuthorization]
-      val Some(resp) = route(app, FakeRequest(GET, "/user/self/name/")
+      val Some(resp) = route(app, FakeRequest(GET, "/v1/user/self/name/")
         .withHeaders(makeAuthHeader(auth))
         .withCSRFToken()
       )
@@ -328,7 +347,7 @@ class OAuthSpec extends PlaySpec
     }
 
     "reject an unauthorized request" in {
-      val Some(resp) = route(app, FakeRequest(GET, "/user/self/name/")
+      val Some(resp) = route(app, FakeRequest(GET, "/v1/user/self/name/")
         .withCSRFToken()
       )
       status(resp) mustEqual BAD_REQUEST
@@ -340,7 +359,7 @@ class OAuthSpec extends PlaySpec
 
     "reject an forged authorized request" in {
       val fakeToken = "7pKNy790TV5lKVjQw3k/pwJmMS8XBhHaLTVaI6ftd5M="
-      val Some(resp) = route(app, FakeRequest(GET, "/user/self/name/")
+      val Some(resp) = route(app, FakeRequest(GET, "/v1/user/self/name/")
         .withHeaders(("Authorization" -> s"Bearer $fakeToken"))
         .withCSRFToken()
       )
@@ -366,7 +385,7 @@ class OAuthSpec extends PlaySpec
         .withCSRFToken()
       )
       status(resp1) mustEqual OK
-      val Some(resp2) = route(app, FakeRequest(GET, "/user/self/name/")
+      val Some(resp2) = route(app, FakeRequest(GET, "/v1/user/self/name/")
         .withHeaders(makeAuthHeader(auth))
         .withCSRFToken()
       )
@@ -393,7 +412,7 @@ class OAuthSpec extends PlaySpec
         .withCSRFToken()
       )
       status(resp1) mustEqual OK
-      val Some(resp2) = route(app, FakeRequest(GET, "/user/self/name/")
+      val Some(resp2) = route(app, FakeRequest(GET, "/v1/user/self/name/")
         .withHeaders(makeAuthHeader(auth))
         .withCSRFToken()
       )
@@ -411,7 +430,7 @@ class OAuthSpec extends PlaySpec
         .withHeaders(makeAuthHeader(email, password))
       ).get).as[UserAuthorization]
       Thread.sleep((1 hour).toMillis)
-      val Some(resp) = route(app, FakeRequest(GET, "/user/self/name/")
+      val Some(resp) = route(app, FakeRequest(GET, "/v1/user/self/name/")
         .withHeaders(makeAuthHeader(auth)))
       status(resp) mustEqual UNAUTHORIZED
       headers(resp) must contain ("WWW-Authenticate" ->
@@ -448,8 +467,7 @@ class OAuthSpec extends PlaySpec
         .withHeaders(makeAuthHeader(users(0)._1.email, users(0)._2))
         .withCSRFToken()
       ).get).as[UserAuthorization]
-      val fakeToken = (auth.accessToken.charAt(0)+1) +
-        auth.accessToken.substring(1);
+      val fakeToken = s"${auth.accessToken.charAt(0)+1}${auth.accessToken.substring(1)}";
       val Some(resp) = route(app, FakeRequest(DELETE, "/authorize/")
         .withJsonBody(Json.parse(s"""|{
                                      |  "grant_type": "refresh_token",
@@ -493,8 +511,7 @@ class OAuthSpec extends PlaySpec
         .withHeaders(makeAuthHeader(users(0)._1.email, users(0)._2))
         .withCSRFToken()
       ).get).as[UserAuthorization]
-      val fakeToken = (auth.refreshToken.charAt(0)+1) +
-        auth.refreshToken.substring(1);
+      val fakeToken = s"${auth.refreshToken.charAt(0)+1}${auth.refreshToken.substring(1)}";
       val Some(resp) = route(app, FakeRequest(DELETE, "/authorize/")
         .withJsonBody(Json.parse(s"""|{
                                      |  "grant_type": "refresh_token",

+ 11 - 7
shared/shared/src/main/scala/com/weEat/shared/models/FoodNode.scala

@@ -116,6 +116,7 @@ sealed trait RecipeNode extends FoodNode {
   def defaultUnitType: UnitType
   def ingredients: Seq[Ingredient]
   def steps: Seq[String]
+  def source: Option[String]
 
   def numServings: Float = stdQties / stdQtiesPServing
   def servingSizeInGrams: Float = stdQtiesPServing * 100
@@ -130,7 +131,8 @@ sealed trait RecipeNode extends FoodNode {
     ingredients,
     steps,
     density,
-    massPerUnit
+    massPerUnit,
+    source
   )
 
   def nutrient(num: String) = ingredients.map({ ingr =>
@@ -163,7 +165,8 @@ case class RecipeNodeNoId(
   val ingredients: Seq[Ingredient],
   val steps: Seq[String],
   override val density: Option[Float],
-  override val massPerUnit: Option[Float]
+  override val massPerUnit: Option[Float],
+  val source: Option[String]
 ) extends RecipeNode
 
 case class RecipeNodeId(
@@ -176,7 +179,8 @@ case class RecipeNodeId(
   val ingredients: Seq[Ingredient],
   val steps: Seq[String],
   override val density: Option[Float],
-  override val massPerUnit: Option[Float]
+  override val massPerUnit: Option[Float],
+  val source: Option[String]
 ) extends RecipeNode with FoodNodeId {
   implicit override val ctx = com.weEat.shared.ctx
 }
@@ -244,10 +248,10 @@ object USDANodeNoId {
     None,
     usda.foodNutrients
       .find(_.nutrientId == kcalNutrientId)
-      .map(_.value)
+      .flatMap(_.value)
       .getOrElse(0.0f),
     usda.foodNutrients
-      .map(x => (x.nutrientNumber, x.value)).toMap
+      .map(x => (x.nutrientNumber, x.value.getOrElse(0.0f))).toMap
   )
 
   import gov.usda.nal.fdc.models._
@@ -302,8 +306,8 @@ object USDANodeNoId {
       id,
       None,
       None,
-      nutr.find(_.nutrientId == kcalNutrientId).map(_.value).getOrElse(0.0f),
-      nutr.map(x => (x.nutrientNumber, x.value)).toMap
+      nutr.find(_.nutrientId == kcalNutrientId).flatMap(_.value).getOrElse(0.0f),
+      nutr.map(x => (x.nutrientNumber, x.value.getOrElse(0.0f))).toMap
     )
     case BrandedFoodItem(id, _, desc, _, _, _, _, _, _, _, _, servSize, servUnit,
       _, nutr, _, _, _) => USDANodeNoId(

+ 9 - 8
shared/shared/src/main/scala/com/weEat/shared/models/MeasureUnit.scala

@@ -78,18 +78,19 @@ object MeasureUnit {
   def apply(i: Int) = units(i)
 
   private def matchDegree(haystack: String)(needle: String) = {
-    val startsWithNeedle = s"$needle[^\\w]".r
-    val containsNeedle = s"[^\\w]$needle[^\\w]".r
-    if (needle == haystack || haystack.startsWith(s"1 $needle")) Some(100000)
-    else if (startsWithNeedle.findFirstIn(haystack).nonEmpty) Some(10000)
-    else if (containsNeedle.findFirstIn(haystack).nonEmpty)
-      Some(haystack.length - needle.length)
+    val startsWithNeedle = s"$needle[^\\s]".r
+    val containsNeedle = s"[^\\s]$needle[^\\s]".r
+    if (needle == haystack || haystack.startsWith(s"1 $needle") || haystack.startsWith(s"${needle}s")) Some(100000)
+    if (needle.size > 1 && haystack.startsWith(s"${needle}")) Some(10000 - Math.abs(haystack.length - needle.length))
+    // else if (containsNeedle.findFirstIn(haystack).nonEmpty)
+    //   Some(10000 - Math.abs(haystack.length - needle.length))
     else None
   }
 
-  def guessUnit(str: String) = {
+  def guessUnit(str: String): Option[MeasureUnit] = {
     val matchFn = matchDegree(str.toLowerCase)(_)
-    units.maxByOption(u => u.lNames.maxByOption(matchFn).flatMap(matchFn))
+    val bestMatch = units.maxBy((u) => u.lNames.map(matchFn).max)
+    bestMatch.lNames.map(matchFn).max.map((_) => bestMatch)
   }
 
   def closestPair[T <: ParsedMeasure](seq: Seq[T]): Option[T] =

+ 55 - 0
shared/shared/src/test/scala/com/weEat/shared/models/MeasureUnitTest.scala

@@ -0,0 +1,55 @@
+import com.weEat.shared.models._
+import org.scalatest._
+
+class MeasureUnitTest() extends WordSpec with MustMatchers {
+
+  "the MeasureUnit guess function" when {
+    "given known measurements correctly" should {
+      val units = Map(
+        ("cup" -> CupUS),
+        ("Cup" -> CupUS),
+        ("cups" -> CupUS),
+        ("Cups" -> CupUS),
+        ("tablespoons" -> TablespoonUS),
+        ("tablespoon" -> TablespoonUS),
+        ("Tablespoons" -> TablespoonUS),
+        ("Tablespoon" -> TablespoonUS),
+        ("teaspoon" -> TeaspoonUS),
+        ("teaspoons" -> TeaspoonUS),
+        ("Teaspoon" -> TeaspoonUS),
+        ("Teaspoons" -> TeaspoonUS),
+        ("Whole" -> Count),
+        ("individual school container" -> Count),
+      )
+
+      for ((str, unit) <- units)
+        testParser(str, unit)
+
+      def testParser(input: String, correct: MeasureUnit) =
+        s"parse '$input' into $correct" in {
+          MeasureUnit.guessUnit(input) mustEqual Some(correct)
+        }
+    }
+
+    "given known bad measurements" when {
+      val giberish = Seq(
+        "large",
+        "cloves",
+        "pea",
+        "Quantity not specified",
+        "Guideline amount per cup of hot cereal",
+        "slice",
+        "ring",
+        "croissant"
+      )
+
+      for (str <- giberish)
+        testParser(str)
+
+      def testParser(input: String) =
+        s"parse '$input' into None" in {
+          MeasureUnit.guessUnit(input) mustEqual None
+        }
+    }
+  }
+}