Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

#[222] No info to user about invalid regex #253

Closed
wants to merge 17 commits into from
90 changes: 58 additions & 32 deletions core/src/main/scala/codesearch/core/search/Search.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +9,66 @@ import cats.syntax.option._
import codesearch.core.config.{Config, SnippetConfig}
import codesearch.core.index.directory.СindexDirectory
import codesearch.core.index.repository.Extensions
import codesearch.core.search.Search.{CSearchPage, CSearchResult, CodeSnippet, Package, PackageResult, snippetConfig}
import codesearch.core.regex.PreciseMatch
import codesearch.core.regex.space.SpaceInsensitiveString
import codesearch.core.search.Search.{CSearchResult, CodeSnippet, Package, PackageResult, snippetConfig}
import codesearch.core.search.SnippetsGrouper.SnippetInfo
import codesearch.core.util.Helper.readFileAsync
import fs2.{Pipe, Stream}
import io.chrisdavenport.log4cats.SelfAwareStructuredLogger
import io.chrisdavenport.log4cats.slf4j.Slf4jLogger
import codesearch.core.regex.RegexConstructor

import scala.sys.process.Process
import scala.sys.process.{Process, ProcessLogger}
import com.google.re2j._

import scala.util.{Success, Try}

trait Search {

protected def cindexDir: СindexDirectory
protected def extensions: Extensions
protected val logger: SelfAwareStructuredLogger[IO] = Slf4jLogger.unsafeCreate[IO]

def checkRegexpForValid(regexp: String): Try[Pattern] = {
Try(Pattern.compile(regexp))
}

def search(request: SearchRequest): IO[CSearchPage] = {
for {
lines <- csearch(request)
results <- Stream
.emits(lines)
.through(SnippetsGrouper.groupLines(snippetConfig))
.drop(snippetConfig.pageSize * (request.page - 1))
.take(snippetConfig.pageSize)
.evalMap(createSnippet)
.through(groupByPackage)
.compile
.toList
} yield CSearchPage(results.sortBy(_.pack.name), lines.size)
checkRegexpForValid(request.query) match {
Kabowyad marked this conversation as resolved.
Show resolved Hide resolved
case Success(_) => {
val entity = csearch(request)
for {
results <- Stream
.emits(entity.lists)
.through(SnippetsGrouper.groupLines(snippetConfig))
.drop(snippetConfig.pageSize * (request.page - 1))
.take(snippetConfig.pageSize)
.evalMap(createSnippet)
.through(groupByPackage)
.compile
.toList
} yield CSearchPage(results.sortBy(_.pack.name), entity.lists.size, ErrorResponse(""))
}
case scala.util.Failure(exception) => {
Kabowyad marked this conversation as resolved.
Show resolved Hide resolved
val message = exception.getMessage
IO(
CSearchPage(Seq.empty[Search.PackageResult],
0,
ErrorResponse(message.substring(0, 1).toUpperCase + message.substring(1, message.size))))
Copy link
Contributor

@kamilongus kamilongus May 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use .lenght instead .size on strings.
You can make a more elegant code without .substring concatination.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use .lenght instead .size on strings.

Does Scala have a linter that catches those things? (Also it's .length)

Copy link
Contributor

@kamilongus kamilongus May 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neongreen
Only IDE inspection support.
image
Or what did you mean?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant a linter that we can run during CI.

I have just tried to run SonarQube on our code, but it didn't find the message.size thing :(

}
}
}

private def csearch(request: SearchRequest): SearchByIndexResult = {
val indexDir = cindexDir.indexDirAs[String]
val env = ("CSEARCHINDEX", indexDir)
var stderr = new String
val log = ProcessLogger((o: String) => o, (e: String) => stderr = e)
val test = for {
_ <- logger.debug(s"running CSEARCHINDEX=$indexDir ${arguments(request).mkString(" ")}")
results <- IO((Process(arguments(request), None, env) #| Seq("head", "-1001")).lineStream_!(log).toList)
} yield SearchByIndexResult(results, ErrorResponse(stderr))
test.unsafeRunSync()
}

/**
Expand Down Expand Up @@ -65,16 +96,6 @@ trait Search {
*/
protected def buildRepUrl(packageName: String, version: String): String

private def csearch(request: SearchRequest): IO[List[String]] = {
val indexDir = cindexDir.indexDirAs[String]
val env = ("CSEARCHINDEX", indexDir)

for {
_ <- logger.debug(s"running CSEARCHINDEX=$indexDir ${arguments(request).mkString(" ")}")
results <- IO((Process(arguments(request), None, env) #| Seq("head", "-1001")).lineStream.toList)
} yield results
}

private def arguments(request: SearchRequest): List[String] = {
def extensionsRegex: String = extensions.sourceExtensions.mkString(".*\\.(", "|", ")$")

Expand All @@ -83,8 +104,10 @@ trait Search {
case None => if (request.sourcesOnly) extensionsRegex else ".*"
}

val query: String =
RegexConstructor(request.query, request.insensitive, request.spaceInsensitive, request.preciseMatch)
val query: String = {
val preciseMatch: String = if (request.preciseMatch) PreciseMatch(request.query) else request.query
if (request.spaceInsensitive) SpaceInsensitiveString(preciseMatch) else preciseMatch
}

request.filter match {
case Some(filter) => List("csearch", "-n", "-f", forExtensions, query, filter)
Expand Down Expand Up @@ -150,11 +173,6 @@ object Search {
* @param data code snippets grouped by package
* @param total number of total matches
*/
final case class CSearchPage(
data: Seq[PackageResult],
total: Int
)

/**
*
* @param relativePath path into package sources
Expand Down Expand Up @@ -200,3 +218,11 @@ object Search {
result: CodeSnippet
)
}
sealed trait Response
final case class SearchByIndexResult(lists: List[String], error: ErrorResponse)
Kabowyad marked this conversation as resolved.
Show resolved Hide resolved
final case class ErrorResponse(message: String) extends Response
final case class CSearchPage(
Kabowyad marked this conversation as resolved.
Show resolved Hide resolved
data: Seq[PackageResult],
total: Int,
error: ErrorResponse
Kabowyad marked this conversation as resolved.
Show resolved Hide resolved
) extends Response
3 changes: 2 additions & 1 deletion project/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ object Builder {
"org.codehaus.janino" % "janino" % "3.0.11",
"com.typesafe.play" %% "play-json" % "2.6.9",
"com.github.mpilquist" %% "simulacrum" % "0.13.0",
"org.typelevel" %% "cats-core" % "1.2.0"
"org.typelevel" %% "cats-core" % "1.2.0",
"com.google.re2j" % "re2j" % "1.2"
)
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import cats.instances.future._
import codesearch.core.db.DefaultDB
import codesearch.core.index.directory.Directory
import codesearch.core.model.DefaultTable
import codesearch.core.search.Search.CSearchPage
import codesearch.core.search.{Search, SearchRequest}
import codesearch.core.search.{CSearchPage, Search, SearchRequest}
import codesearch.core.util.Helper
import com.github.marlonlom.utilities.timeago.TimeAgo
import play.api.mvc.{Action, AnyContent, InjectedController}
Expand Down Expand Up @@ -51,7 +50,7 @@ trait SearchController[V <: DefaultTable] { self: InjectedController =>

db.updated.flatMap { updated =>
searchEngine.search(searchRequest) map {
case CSearchPage(results, total) =>
case CSearchPage(results, total, errorResponse) =>
Ok(
views.html.searchResults(
updated = TimeAgo.using(updated.getTime),
Expand All @@ -66,7 +65,8 @@ trait SearchController[V <: DefaultTable] { self: InjectedController =>
page = searchRequest.page,
totalMatches = total,
callURI = searchRequest.callURI(host).toString,
lang = lang
lang = lang,
errorMessageT = errorResponse.message
)
)
} unsafeToFuture
Expand Down
2 changes: 1 addition & 1 deletion web-server/app/views/search.scala.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
}

@wrapper(s"Codesearch | $lang", headExtra){
@searchBox(s"/$lang/search", "", None, None, insensitive = false, space = false, precise = false, sources = true)
@searchBox(s"/$lang/search", "", None, None, insensitive = false, space = false, precise = false, sources = true, "")
@docFrame()
}
15 changes: 5 additions & 10 deletions web-server/app/views/searchBox.scala.html
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
@(
actSearch: String,
query: String,
filter: Option[String],
filePath: Option[String],
insensitive: Boolean,
space: Boolean,
precise: Boolean,
sources: Boolean
)
@(actSearch: String, query: String, filter: Option[String], filePath: Option[String], insensitive: Boolean,
space: Boolean, precise: Boolean, sources: Boolean, errorMessageT: String)

<section class="container-fluid">
<div class="col-lg-6 mx-auto p-3 h7 justify-content-center" style="position: relative;">
<form>
<section class="warning warning-query">Please provide a string to search for.</section>
@if(errorMessageT.nonEmpty) {
<section class="error error-query" style="color:#f44336;">️⚠️ @{errorMessageT}</section>
}
<div class="input-group mb-2">
<input type="text" class="form-control" name="query" value="@query" class="query">
<div class="input-group-append">
Expand Down
23 changes: 12 additions & 11 deletions web-server/app/views/searchResults.scala.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,18 @@
page: Int,
totalMatches: Int,
callURI: String,
lang: String
lang: String,
errorMessageT: String
)

@headExtra = {
<script src="@routes.Assets.versioned("prismjs.js")"></script>
<link rel="stylesheet" type="text/css" href="@routes.Assets.versioned("css/prismjs.css")">
<link rel="stylesheet" type="text/css" href="@routes.Assets.versioned("css/search.css")">
}
@headExtra = {
<script src="@routes.Assets.versioned("prismjs.js")"></script>
<link rel="stylesheet" type="text/css" href="@routes.Assets.versioned("css/prismjs.css")">
<link rel="stylesheet" type="text/css" href="@routes.Assets.versioned("css/search.css")">
}

@wrapper(s"Codesearch | $lang", headExtra) {
@searchBox(s"/$lang/search", query, filter, filePath, insensitive, space, precise, sources)
@resultFrame(lang, insensitive, space, precise, query, updated, packages, totalMatches)
@pagination(page, totalMatches, callURI)
}
@wrapper(s"Codesearch | $lang", headExtra) {
@searchBox(s"/$lang/search", query, filter, filePath, insensitive, space, precise, sources, errorMessageT)
@resultFrame(lang, insensitive, space, precise, query, updated, packages, totalMatches)
@pagination(page, totalMatches, callURI)
}