Refactor based on review suggestions

Author: pavlovicisidora

src/main/scala/io/lambdaworks/detection/UrlDetector.scala

@@ -10,14 +10,15 @@ import org.apache.commons.validator.routines.EmailValidator
 import scala.collection.immutable.SortedSet
 import scala.jdk.CollectionConverters._
 import scala.util.matching.Regex
+
 import UrlDetector._
 
 final class UrlDetector private (
-                                  options: UrlDetectorOptions,
-                                  allowed: Option[NonEmptySet[Host]],
-                                  denied: Option[NonEmptySet[Host]],
-                                  emailValidator: EmailValidator
-                                ) {
+  options: UrlDetectorOptions,
+  allowed: Option[NonEmptySet[Host]],
+  denied: Option[NonEmptySet[Host]],
+  emailValidator: EmailValidator
+) {
 
   private val allowedWithoutWww: Option[NonEmptySet[Host]] =
     allowed.flatMap(allowed => NonEmptySet.fromSet(allowed.toSortedSet.flatMap(removeWwwSubdomain)))
@@ -39,44 +40,11 @@ final class UrlDetector private (
       .detect()
       .asScala
       .toList
-      .map { lUrl =>
-        val rawUrl = lUrl.toString
-        val cleanedUrl = cleanUrlForBracketMatch(content, rawUrl)
-        AbsoluteUrl.parse(sanitize(cleanedUrl))
-      }
+      .map(url => AbsoluteUrl.parse(sanitize(cleanUrlForBracketMatch(content, url.toString))))
       .filter(url => allowedUrl(url) && notEmail(url) && validTopLevelDomain(url))
       .toSet
   }
 
-  private def cleanUrlForBracketMatch(originalText: String, urlStr: String): String = {
-    val allowedSpecialChars: Set[Char] = Set(
-      '-', '.', '_', '~', ':', '/', '?', '#', '[', ']', '@',
-      '!', '$', '&', '\'', '(', ')', '*', '+', ',', ';', '=', '%'
-    )
-
-    def isAllowedUrlChar(c: Char): Boolean = {
-      c.isLetterOrDigit || allowedSpecialChars.contains(c)
-    }
-
-    val startIndexOpt = Option(originalText.indexOf(urlStr)).filter(_ >= 0)
-
-    startIndexOpt match {
-      case None => urlStr
-
-      case Some(startIndex) =>
-        val extendedUrl = originalText
-          .substring(startIndex)
-          .takeWhile(isAllowedUrlChar)
-
-        val emptyParensPattern = """\(\)[^()]*""".r
-
-        emptyParensPattern.findFirstMatchIn(extendedUrl) match {
-          case Some(m) => extendedUrl.substring(0, m.start)
-          case None    => extendedUrl
-        }
-    }
-  }
-
   /**
    * Method that creates a [[io.lambdaworks.detection.UrlDetector]] with a set of hosts to allow.
    *
@@ -119,6 +87,22 @@ final class UrlDetector private (
   private def allowedUrl(url: AbsoluteUrl): Boolean =
     allowedWithoutWww.forall(containsHost(_, url)) && deniedWithoutWww.forall(!containsHost(_, url))
 
+  private def cleanUrlForBracketMatch(originalText: String, urlStr: String): String = {
+    def isAllowedUrlChar(c: Char): Boolean =
+      c.isLetterOrDigit || AllowedSpecialChars.contains(c)
+
+    Option(originalText.indexOf(urlStr)).filter(_ >= 0).fold(urlStr) { startIndex =>
+      val extendedUrl = originalText
+        .substring(startIndex)
+        .takeWhile(isAllowedUrlChar)
+
+      EmptyParensRegex
+        .findFirstMatchIn(extendedUrl)
+        .map(m => extendedUrl.substring(0, m.start))
+        .getOrElse(extendedUrl)
+    }
+  }
+
   private def containsHost(hosts: NonEmptySet[Host], url: AbsoluteUrl): Boolean =
     hosts.exists(host => host.subdomain.fold(host.apexDomain.exists(url.apexDomain.contains))(_ => host == url.host))
 
@@ -164,7 +148,12 @@ object UrlDetector {
    */
   lazy val default: UrlDetector = UrlDetector(UrlDetectorOptions.Default)
 
-  private final val SanitizeRegex: Regex = "[,!-.`/]+$".r
+  private final val AllowedSpecialChars: Set[Char] = Set(
+    '-', '.', '_', '~', ':', '/', '?', '#', '[', ']', '@', '!', '$', '&', '\'', '(', ')', '*', '+', ',', ';', '=', '%'
+  )
+
+  private final val EmptyParensRegex: Regex = """\(\)[^()]*""".r
+  private final val SanitizeRegex: Regex    = "[,!-.`/]+$".r
 
   implicit private[detection] val orderingHost: Ordering[Host] = orderHost.toOrdering
 

src/test/scala/io/lambdaworks/detection/UrlDetectorSpec.scala

@@ -45,7 +45,11 @@ final class UrlDetectorSpec extends AnyFlatSpec with Matchers {
         ),
         (
           "Parse https://learn.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/aa752574(v=vs.85)?redirectedfrom=MSDN",
-          Set(Url.parse("https://learn.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/aa752574(v=vs.85)?redirectedfrom=MSDN"))
+          Set(
+            Url.parse(
+              "https://learn.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/aa752574(v=vs.85)?redirectedfrom=MSDN"
+            )
+          )
         ),
         (
           "192.168.1.3 255.255.1.34 1234.34.34.5 0.0.0.0 192.168.1.257 2.3.4.5",
@@ -289,19 +293,23 @@ final class UrlDetectorSpec extends AnyFlatSpec with Matchers {
             Url.parse("http://test.link/g3WMrh"),
             Url.parse("http://test.link/HWRqhq"),
             Url.parse("http://test.link/KeKy")
-          ),
+          )
         ),
-          (
-            "Parse https://site.com/(v=1.2)",
-            Set(Url.parse("https://site.com/(v=1.2"))
-          ),
         (
-          "Parse https://learn.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/aa752574(v=vs.85)?redirectedfrom=MSDN",
-          Set(Url.parse("https://learn.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/aa752574(v=vs.85)?redirectedfrom=MSDN"))
+          "Parse https://site.com/(v=1.2)",
+          Set(Url.parse("https://site.com/(v=1.2"))
         ),
         (
-          "Parse http://www.website.com/?utm_source=google%5BB%2B%5D&utm_medium=cpc&utm_content=google_ad(B)&utm_campaign=product",
-          Set(Url.parse("http://www.website.com/?utm_source=google%5BB%2B%5D&utm_medium=cpc&utm_content=google_ad(B)&utm_campaign=product"))
+          "Parse https://learn.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/aa752574(v=vs.85)?redirectedfrom=MSDN " +
+            "and http://www.website.com/?utm_source=google%5BB%2B%5D&utm_medium=cpc&utm_content=google_ad(B)&utm_campaign=product",
+          Set(
+            Url.parse(
+              "https://learn.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/aa752574(v=vs.85)?redirectedfrom=MSDN"
+            ),
+            Url.parse(
+              "http://www.website.com/?utm_source=google%5BB%2B%5D&utm_medium=cpc&utm_content=google_ad(B)&utm_campaign=product"
+            )
+          )
         )
       )