From 0bb87a4a8d84c197c07a6358c0c53a9b4e263b2d Mon Sep 17 00:00:00 2001
From: Jonas Natten <jonas.natten@knowit.no>
Date: Thu, 5 Dec 2024 15:15:52 +0100
Subject: [PATCH] Stop allowing whitespace only strings in `NonEmptyString`

This patch fixes a problem where search queries would return 500 if you
passed a string with only spaces.
---
 .../no/ndla/network/tapir/NonEmptyString.scala | 18 ++++++++++++------
 .../network/tapir/NonEmptyStringTest.scala     | 17 +++++++++++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/network/src/main/scala/no/ndla/network/tapir/NonEmptyString.scala b/network/src/main/scala/no/ndla/network/tapir/NonEmptyString.scala
index 03f71cefc..aa8b65b48 100644
--- a/network/src/main/scala/no/ndla/network/tapir/NonEmptyString.scala
+++ b/network/src/main/scala/no/ndla/network/tapir/NonEmptyString.scala
@@ -12,20 +12,26 @@ import io.circe.{Decoder, DecodingFailure, Encoder, FailedCursor, HCursor, Json}
 import sttp.tapir.CodecFormat.TextPlain
 import sttp.tapir.{Codec, CodecFormat, DecodeResult, Schema}
 
-/** Class that cannot be constructed with an empty string (""), therefore it means that if you have one of these the
-  * underlying string is not empty
+/** Class that cannot be constructed with an empty string ("") or a whitespace only string (" "), therefore it means
+  * that if you have one of these the underlying string is not empty.
   */
 class NonEmptyString private (val underlying: String) {
   override def equals(obj: Any): Boolean = obj match {
     case other: NonEmptyString => other.underlying == underlying
+    case other: String         => other == underlying
     case _                     => false
   }
 }
 
 object NonEmptyString {
-  def apply(underlying: String): Option[NonEmptyString]        = fromString(underlying)
-  def fromOptString(s: Option[String]): Option[NonEmptyString] = s.filter(_.nonEmpty).map(f => new NonEmptyString(f))
-  def fromString(s: String): Option[NonEmptyString]            = Option.when(s.nonEmpty)(new NonEmptyString(s))
+  def apply(underlying: String): Option[NonEmptyString] = fromString(underlying)
+
+  private def validateString(underlying: String): Boolean = underlying.trim.nonEmpty
+
+  def fromOptString(s: Option[String]): Option[NonEmptyString] =
+    s.filter(validateString).map(f => new NonEmptyString(f))
+  def fromString(s: String): Option[NonEmptyString] =
+    Option.when(validateString(s))(new NonEmptyString(s))
 
   implicit val schema: Schema[NonEmptyString]            = Schema.string
   implicit val schemaOpt: Schema[Option[NonEmptyString]] = Schema.string.asOption
@@ -60,7 +66,7 @@ object NonEmptyString {
 
   implicit def circeEncoder: Encoder[NonEmptyString] = (a: NonEmptyString) => Json.fromString(a.underlying)
 
-  /** Helpers that should make working with a bit `Option[NonEmptyString]` easier */
+  /** Helpers that should make working with `Option[NonEmptyString]` a bit easier */
   implicit class NonEmptyStringImplicit(self: Option[NonEmptyString]) {
     def underlying: Option[String]                   = self.map(_.underlying)
     def underlyingOrElse(default: => String): String = self.map(_.underlying).getOrElse(default)
diff --git a/network/src/test/scala/no/ndla/network/tapir/NonEmptyStringTest.scala b/network/src/test/scala/no/ndla/network/tapir/NonEmptyStringTest.scala
index 4c200d8f8..d81080c41 100644
--- a/network/src/test/scala/no/ndla/network/tapir/NonEmptyStringTest.scala
+++ b/network/src/test/scala/no/ndla/network/tapir/NonEmptyStringTest.scala
@@ -93,4 +93,21 @@ class NonEmptyStringTest extends UnitSuite {
     failure.message should be(NonEmptyString.parseErrorMessage)
   }
 
+  test("That comparisons works as expected") {
+    val a = NonEmptyString.fromString("hei").get
+    val b = NonEmptyString.fromString("hei").get
+    val c = NonEmptyString.fromString("hallo").get
+
+    a == b should be(true)
+    a == c should be(false)
+    a == "hei" should be(true)
+    a == "hallo" should be(false)
+  }
+
+  test("That only whitespace strings are not parsed as NonEmptyString") {
+    NonEmptyString.fromString(" ") should be(None)
+    NonEmptyString.fromString("  ") should be(None)
+    NonEmptyString.fromString("   ") should be(None)
+  }
+
 }