Skip to content

Commit

Permalink
[TRELLO-2608] Rework some request scanning the whole report table
Browse files Browse the repository at this point in the history
  • Loading branch information
charlescd committed Oct 22, 2024
1 parent 9570294 commit 3ce676c
Show file tree
Hide file tree
Showing 18 changed files with 113 additions and 59 deletions.
7 changes: 4 additions & 3 deletions app/controllers/AuthController.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package controllers

import authentication.CookieAuthenticator
import models.PaginatedResult
import models.UserRole
import orchestrators.AuthOrchestrator
import play.api._
Expand Down Expand Up @@ -62,11 +63,11 @@ class AuthController(
Future.successful(Ok(Json.toJson(request.identity)))
}

def listAuthAttempts(login: Option[String]) =
def listAuthAttempts(login: Option[String], offset: Option[Long], limit: Option[Int]) =
SecuredAction.andThen(WithRole(UserRole.AdminsAndReadOnly)).async(parse.empty) { _ =>
authOrchestrator
.listAuthenticationAttempts(login)
.map(authAttempts => Ok(Json.toJson(authAttempts)))
.listAuthenticationAttempts(login, offset, limit)
.map(authAttempts => Ok(Json.toJson(authAttempts)(PaginatedResult.paginatedResultWrites)))
}

def forgotPassword: Action[JsValue] = IpRateLimitedAction2.async(parse.json) { implicit request =>
Expand Down
20 changes: 13 additions & 7 deletions app/controllers/ReportedPhoneController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import org.apache.pekko.actor.typed
import authentication.Authenticator
import models._
import play.api.Logger
import play.api.libs.json.{JsObject, Json}
import play.api.libs.json.Json
import play.api.mvc.ControllerComponents
import repositories.asyncfiles.AsyncFileRepositoryInterface
import repositories.company.CompanyRepositoryInterface
Expand All @@ -31,7 +31,13 @@ class ReportedPhoneController(
implicit val timeout: org.apache.pekko.util.Timeout = 5.seconds
val logger: Logger = Logger(this.getClass)

def fetchGrouped(q: Option[String], start: Option[String], end: Option[String], offset: Option[Long], limit: Option[Int]) =
def fetchGrouped(
q: Option[String],
start: Option[String],
end: Option[String],
offset: Option[Long],
limit: Option[Int]
) =
SecuredAction.andThen(WithRole(UserRole.AdminsAndReadOnlyAndCCRF)).async { _ =>
reportRepository
.getPhoneReports(q, DateUtils.parseDate(start), DateUtils.parseDate(end), offset, limit)
Expand All @@ -41,14 +47,14 @@ class ReportedPhoneController(
reports
.mapEntities { case ((phone, siretOpt, companyNameOpt, category), count) =>
Json.obj(
"phone" -> phone,
"siret" -> siretOpt,
"phone" -> phone,
"siret" -> siretOpt,
"companyName" -> companyNameOpt,
"category" -> category,
"count" -> count
"category" -> category,
"count" -> count
)
}
)(PaginatedResult.paginatedResultWrites[JsObject])
)(PaginatedResult.paginatedResultWrites)
)
)
}
Expand Down
8 changes: 7 additions & 1 deletion app/controllers/WebsiteController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,13 @@ class WebsiteController(
} yield Ok(resultAsJson)
}

def fetchUnregisteredHost(host: Option[String], start: Option[String], end: Option[String], offset: Option[Long], limit: Option[Int]) =
def fetchUnregisteredHost(
host: Option[String],
start: Option[String],
end: Option[String],
offset: Option[Long],
limit: Option[Int]
) =
SecuredAction.andThen(WithRole(UserRole.AdminsAndReadOnlyAndCCRF)).async { _ =>
websitesOrchestrator
.fetchUnregisteredHost(host, start, end, offset, limit)
Expand Down
5 changes: 1 addition & 4 deletions app/models/report/ReportFilter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,7 @@ object ReportFilter {
end = mapper.timeWithLocalDateRetrocompatEndOfDay("end"),
category = mapper.string("category"),
companyIds = mapper.seq("companyIds").map(UUID.fromString),
status = {
val statuses = mapper.seq("status").map(ReportStatus.withName)
if (statuses.isEmpty) ReportStatus.values else statuses
},
status = mapper.seq("status").map(ReportStatus.withName),
details = mapper.string("details", trimmed = true),
description = mapper.string("description", trimmed = true),
hasForeignCountry = mapper.boolean("hasForeignCountry"),
Expand Down
7 changes: 4 additions & 3 deletions app/models/report/ReportStatus.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@ object ReportStatus extends PlayEnum[ReportStatus] {

implicit class ReportStatusOps(reportStatus: ReportStatus) {
def isFinal: Boolean =
Seq(MalAttribue, ConsulteIgnore, NonConsulte, Infonde, PromesseAction, InformateurInterne, NA, SuppressionRGPD).contains(
reportStatus
)
Seq(MalAttribue, ConsulteIgnore, NonConsulte, Infonde, PromesseAction, InformateurInterne, NA, SuppressionRGPD)
.contains(
reportStatus
)

def isNotFinal = !isFinal
}
Expand Down
9 changes: 7 additions & 2 deletions app/orchestrators/AuthOrchestrator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import cats.syntax.option._
import config.TokenConfiguration
import controllers.error.AppError
import controllers.error.AppError._
import models.PaginatedResult
import models.User
import models.UserRole
import models.auth._
Expand Down Expand Up @@ -217,9 +218,13 @@ class AuthOrchestrator(
_ = logger.debug(s"Auth attempts count check successful")
} yield ()

def listAuthenticationAttempts(login: Option[String]): Future[Seq[AuthAttempt]] =
def listAuthenticationAttempts(
login: Option[String],
offset: Option[Long],
limit: Option[Int]
): Future[PaginatedResult[AuthAttempt]] =
for {
authAttempts <- authAttemptRepository.listAuthAttempts(login)
authAttempts <- authAttemptRepository.listAuthAttempts(login, offset, limit)
} yield authAttempts

}
Expand Down
3 changes: 2 additions & 1 deletion app/orchestrators/WebsitesOrchestrator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,8 @@ class WebsitesOrchestrator(
host: Option[String],
start: Option[String],
end: Option[String],
offset: Option[Long], limit: Option[Int]
offset: Option[Long],
limit: Option[Int]
): Future[PaginatedResult[WebsiteHostCount]] =
repository
.getUnkonwnReportCountByHost(host, DateUtils.parseDate(start), DateUtils.parseDate(end), offset, limit)
Expand Down
26 changes: 16 additions & 10 deletions app/repositories/authattempt/AuthAttemptRepository.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package repositories.authattempt

import models.PaginatedResult
import models.auth.AuthAttempt
import models.auth.AuthAttemptFilter
import play.api.Logger
Expand Down Expand Up @@ -47,15 +48,20 @@ class AuthAttemptRepository(
.result
)

override def listAuthAttempts(login: Option[String]): Future[Seq[AuthAttempt]] = db
.run(
table
.filterOpt(login) { case (table, login) =>
table.login like s"%${login}%"
}
.sortBy(_.timestamp.desc)
.take(1000)
.result
)
override def listAuthAttempts(
login: Option[String],
offset: Option[Long],
limit: Option[Int]
): Future[PaginatedResult[AuthAttempt]] =
table
.filterOpt(login) { case (table, login) =>
table.login like s"%${login}%"
}
.sortBy(_.timestamp.desc)
.withPagination(db)(
maybeOffset = offset,
maybeLimit = limit,
maybePreliminaryAction = None
)

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package repositories.authattempt

import models.PaginatedResult
import models.auth.AuthAttempt
import models.auth.AuthAttemptFilter
import repositories.CRUDRepositoryInterface
Expand All @@ -12,5 +13,9 @@ trait AuthAttemptRepositoryInterface extends CRUDRepositoryInterface[AuthAttempt

def countAuthAttempts(filter: AuthAttemptFilter): Future[Int]

def listAuthAttempts(login: Option[String]): Future[Seq[AuthAttempt]]
def listAuthAttempts(
login: Option[String],
offset: Option[Long],
limit: Option[Int]
): Future[PaginatedResult[AuthAttempt]]
}
13 changes: 10 additions & 3 deletions app/repositories/report/ReportRepository.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import models.report.ReportResponseType.ACCEPTED
import models.report._
import models.report.reportmetadata.ReportMetadata
import models.report.reportmetadata.ReportWithMetadata
import repositories.{CRUDRepository, PaginateOps}
import repositories.CRUDRepository
import repositories.PaginateOps
import repositories.PostgresProfile.api._
import repositories.barcode.BarcodeProductTable
import repositories.company.CompanyTable
Expand Down Expand Up @@ -454,7 +455,13 @@ class ReportRepository(override val dbConfig: DatabaseConfig[JdbcProfile])(impli
.result
)

def getPhoneReports(q: Option[String], start: Option[LocalDate], end: Option[LocalDate], offset: Option[Long], limit: Option[Int]): Future[PaginatedResult[((Option[String], Option[SIRET], Option[String], String), Int)]] =
def getPhoneReports(
q: Option[String],
start: Option[LocalDate],
end: Option[LocalDate],
offset: Option[Long],
limit: Option[Int]
): Future[PaginatedResult[((Option[String], Option[SIRET], Option[String], String), Int)]] =
table
.filter(_.phone.isDefined)
.filterOpt(q) { case (table, p) =>
Expand All @@ -467,7 +474,7 @@ class ReportRepository(override val dbConfig: DatabaseConfig[JdbcProfile])(impli
table.creationDate < ZonedDateTime.of(end, LocalTime.MAX, ZoneOffset.UTC.normalized()).toOffsetDateTime
}
.groupBy(a => (a.phone, a.companySiret, a.companyName, a.category))
.map { case (a, b) => (a, b.length)}
.map { case (a, b) => (a, b.length) }
.sortBy(_._2.desc)
.withPagination(db)(
maybeOffset = offset,
Expand Down
8 changes: 7 additions & 1 deletion app/repositories/report/ReportRepositoryInterface.scala
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,13 @@ trait ReportRepositoryInterface extends CRUDRepositoryInterface[Report] {

def getPhoneReports(start: Option[LocalDate], end: Option[LocalDate]): Future[List[Report]]

def getPhoneReports(q: Option[String], start: Option[LocalDate], end: Option[LocalDate], offset: Option[Long], limit: Option[Int]): Future[PaginatedResult[((Option[String], Option[SIRET], Option[String], String), Int)]]
def getPhoneReports(
q: Option[String],
start: Option[LocalDate],
end: Option[LocalDate],
offset: Option[Long],
limit: Option[Int]
): Future[PaginatedResult[((Option[String], Option[SIRET], Option[String], String), Int)]]

def reportsCountBySubcategories(
userRole: UserRole,
Expand Down
16 changes: 9 additions & 7 deletions app/repositories/website/WebsiteRepository.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import models.website.Website
import models.website.WebsiteId
import play.api.Logger
import repositories.PostgresProfile.api._
import repositories.{PaginateOps, TypedCRUDRepository}
import repositories.PaginateOps
import repositories.TypedCRUDRepository
import repositories.company.CompanyTable
import repositories.report.ReportTable
import repositories.website.WebsiteColumnType._
Expand Down Expand Up @@ -188,7 +189,7 @@ class WebsiteRepository(
def getUnkonwnReportCountByHost(
host: Option[String],
start: Option[LocalDate],
end: Option[LocalDate],
end: Option[LocalDate]
): Future[List[(String, Int)]] = db
.run(
WebsiteTable.table
Expand All @@ -212,11 +213,12 @@ class WebsiteRepository(
)

def getUnkonwnReportCountByHost(
host: Option[String],
start: Option[LocalDate],
end: Option[LocalDate],
offset: Option[Long], limit: Option[Int]
): Future[PaginatedResult[(String, Int)]] =
host: Option[String],
start: Option[LocalDate],
end: Option[LocalDate],
offset: Option[Long],
limit: Option[Int]
): Future[PaginatedResult[(String, Int)]] =
WebsiteTable.table
.filter(t => host.fold(true.bind)(h => t.host like s"%${h}%"))
.filter(x => x.companyId.isEmpty && x.companyCountry.isEmpty)
Expand Down
11 changes: 6 additions & 5 deletions app/repositories/website/WebsiteRepositoryInterface.scala
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,12 @@ trait WebsiteRepositoryInterface extends TypedCRUDRepositoryInterface[Website, W
): Future[List[(String, Int)]]

def getUnkonwnReportCountByHost(
host: Option[String],
start: Option[LocalDate],
end: Option[LocalDate],
offset: Option[Long], limit: Option[Int]
): Future[PaginatedResult[(String, Int)]]
host: Option[String],
start: Option[LocalDate],
end: Option[LocalDate],
offset: Option[Long],
limit: Option[Int]
): Future[PaginatedResult[(String, Int)]]

def listNotAssociatedToCompany(host: String): Future[Seq[Website]]

Expand Down
2 changes: 2 additions & 0 deletions conf/db/migration/default/V38__optimizations.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
create index if not exists idx_auth_attempts_login
on auth_attempts (login);
2 changes: 1 addition & 1 deletion conf/routes
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ GET /api/current-user controller
POST /api/authenticate/password/forgot controllers.AuthController.forgotPassword()
POST /api/authenticate/password/reset controllers.AuthController.resetPassword(token: java.util.UUID)
POST /api/account/password controllers.AuthController.changePassword()
GET /api/auth-attempts controllers.AuthController.listAuthAttempts(login: Option[String])
GET /api/auth-attempts controllers.AuthController.listAuthAttempts(login: Option[String], offset: Option[Long], limit: Option[Int])

# Accesses API
GET /api/accesses/connected-user controllers.CompanyAccessController.myCompanies()
Expand Down
12 changes: 6 additions & 6 deletions test/controllers/AuthControllerSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,12 @@ class AuthControllerSpec(implicit ee: ExecutionEnv)

val result = for {
res <- route(app, request).get
authAttempts <- authAttemptRepository.listAuthAttempts(Some(login))
authAttempts <- authAttemptRepository.listAuthAttempts(Some(login), None, None)
} yield (res, authAttempts)

Helpers.status(result.map(_._1)) must beEqualTo(OK)

val authAttempts = Await.result(result.map(_._2), Duration.Inf)
val authAttempts = Await.result(result.map(_._2), Duration.Inf).entities
authAttempts.length shouldEqual 1
authAttempts.headOption.map(_.login) shouldEqual Some(login)
authAttempts.headOption.flatMap(_.isSuccess) shouldEqual (Some(true))
Expand Down Expand Up @@ -137,15 +137,15 @@ class AuthControllerSpec(implicit ee: ExecutionEnv)

val result = for {
res <- route(app, request).get
authAttempts <- authAttemptRepository.listAuthAttempts(Some(login))
authAttempts <- authAttemptRepository.listAuthAttempts(Some(login), None, None)
} yield (res, authAttempts)

Helpers.status(result.map(_._1)) must beEqualTo(UNAUTHORIZED)
Helpers.contentAsJson(result.map(_._1)) must beEqualTo(
Json.toJson(failedAuthenticationErrorPayload)
)

val authAttempts = Await.result(result.map(_._2), Duration.Inf)
val authAttempts = Await.result(result.map(_._2), Duration.Inf).entities
authAttempts.length shouldEqual 1
authAttempts.headOption.map(_.login) shouldEqual Some(login)
authAttempts.headOption.flatMap(_.isSuccess) shouldEqual (Some(false))
Expand All @@ -163,15 +163,15 @@ class AuthControllerSpec(implicit ee: ExecutionEnv)

val result = for {
res <- route(app, request).get
authAttempts <- authAttemptRepository.listAuthAttempts(Some(login))
authAttempts <- authAttemptRepository.listAuthAttempts(Some(login), None, None)
} yield (res, authAttempts)

Helpers.status(result.map(_._1)) must beEqualTo(UNAUTHORIZED)
Helpers.contentAsJson(result.map(_._1)) must beEqualTo(
Json.toJson(failedAuthenticationErrorPayload)
)

val authAttempts = Await.result(result.map(_._2), Duration.Inf)
val authAttempts = Await.result(result.map(_._2), Duration.Inf).entities
authAttempts.length shouldEqual 1
authAttempts.headOption.map(_.login) shouldEqual Some(login)
authAttempts.headOption.flatMap(_.isSuccess) shouldEqual (Some(false))
Expand Down
5 changes: 3 additions & 2 deletions test/controllers/WebsiteControllerSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,9 @@ The fetch unregistered host endpoint should
"""

def e1 = {
val request = FakeRequest(GET, routes.WebsiteController.fetchUnregisteredHost(None, None, None, None, None).toString)
.withAuthCookie(adminUser.email, components.cookieAuthenticator)
val request =
FakeRequest(GET, routes.WebsiteController.fetchUnregisteredHost(None, None, None, None, None).toString)
.withAuthCookie(adminUser.email, components.cookieAuthenticator)
val result = route(app, request).get
status(result) must beEqualTo(OK)
val content = contentAsJson(result).toString
Expand Down
Loading

0 comments on commit 3ce676c

Please sign in to comment.