From 3ce676cf125c1da31fb3561c0f84e2f2fa0b6286 Mon Sep 17 00:00:00 2001 From: Charles Dufour Date: Tue, 22 Oct 2024 15:35:25 +0200 Subject: [PATCH] [TRELLO-2608] Rework some request scanning the whole report table --- app/controllers/AuthController.scala | 7 ++--- app/controllers/ReportedPhoneController.scala | 20 +++++++++----- app/controllers/WebsiteController.scala | 8 +++++- app/models/report/ReportFilter.scala | 5 +--- app/models/report/ReportStatus.scala | 7 ++--- app/orchestrators/AuthOrchestrator.scala | 9 +++++-- app/orchestrators/WebsitesOrchestrator.scala | 3 ++- .../authattempt/AuthAttemptRepository.scala | 26 ++++++++++++------- .../AuthAttemptRepositoryInterface.scala | 7 ++++- .../report/ReportRepository.scala | 13 +++++++--- .../report/ReportRepositoryInterface.scala | 8 +++++- .../website/WebsiteRepository.scala | 16 +++++++----- .../website/WebsiteRepositoryInterface.scala | 11 ++++---- .../migration/default/V38__optimizations.sql | 2 ++ conf/routes | 2 +- test/controllers/AuthControllerSpec.scala | 12 ++++----- test/controllers/WebsiteControllerSpec.scala | 5 ++-- .../report/ReportRepositoryMock.scala | 11 ++++++-- 18 files changed, 113 insertions(+), 59 deletions(-) create mode 100644 conf/db/migration/default/V38__optimizations.sql diff --git a/app/controllers/AuthController.scala b/app/controllers/AuthController.scala index 8b8fb1d9..bbb4ba35 100644 --- a/app/controllers/AuthController.scala +++ b/app/controllers/AuthController.scala @@ -1,6 +1,7 @@ package controllers import authentication.CookieAuthenticator +import models.PaginatedResult import models.UserRole import orchestrators.AuthOrchestrator import play.api._ @@ -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 => diff --git a/app/controllers/ReportedPhoneController.scala b/app/controllers/ReportedPhoneController.scala index 00f408be..6fb18e5d 100644 --- a/app/controllers/ReportedPhoneController.scala +++ b/app/controllers/ReportedPhoneController.scala @@ -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 @@ -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) @@ -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) ) ) } diff --git a/app/controllers/WebsiteController.scala b/app/controllers/WebsiteController.scala index af471e7a..c924cadf 100644 --- a/app/controllers/WebsiteController.scala +++ b/app/controllers/WebsiteController.scala @@ -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) diff --git a/app/models/report/ReportFilter.scala b/app/models/report/ReportFilter.scala index 8ef330dc..305c61d3 100644 --- a/app/models/report/ReportFilter.scala +++ b/app/models/report/ReportFilter.scala @@ -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"), diff --git a/app/models/report/ReportStatus.scala b/app/models/report/ReportStatus.scala index efccd639..bca7e8e1 100644 --- a/app/models/report/ReportStatus.scala +++ b/app/models/report/ReportStatus.scala @@ -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 } diff --git a/app/orchestrators/AuthOrchestrator.scala b/app/orchestrators/AuthOrchestrator.scala index 8313eede..af322800 100644 --- a/app/orchestrators/AuthOrchestrator.scala +++ b/app/orchestrators/AuthOrchestrator.scala @@ -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._ @@ -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 } diff --git a/app/orchestrators/WebsitesOrchestrator.scala b/app/orchestrators/WebsitesOrchestrator.scala index 9e5c72c0..cc4f33b7 100644 --- a/app/orchestrators/WebsitesOrchestrator.scala +++ b/app/orchestrators/WebsitesOrchestrator.scala @@ -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) diff --git a/app/repositories/authattempt/AuthAttemptRepository.scala b/app/repositories/authattempt/AuthAttemptRepository.scala index e4e7195d..8f68c8d2 100644 --- a/app/repositories/authattempt/AuthAttemptRepository.scala +++ b/app/repositories/authattempt/AuthAttemptRepository.scala @@ -1,5 +1,6 @@ package repositories.authattempt +import models.PaginatedResult import models.auth.AuthAttempt import models.auth.AuthAttemptFilter import play.api.Logger @@ -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 + ) } diff --git a/app/repositories/authattempt/AuthAttemptRepositoryInterface.scala b/app/repositories/authattempt/AuthAttemptRepositoryInterface.scala index 364d25e9..89d09b21 100644 --- a/app/repositories/authattempt/AuthAttemptRepositoryInterface.scala +++ b/app/repositories/authattempt/AuthAttemptRepositoryInterface.scala @@ -1,5 +1,6 @@ package repositories.authattempt +import models.PaginatedResult import models.auth.AuthAttempt import models.auth.AuthAttemptFilter import repositories.CRUDRepositoryInterface @@ -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]] } diff --git a/app/repositories/report/ReportRepository.scala b/app/repositories/report/ReportRepository.scala index 5bad004e..d4f1de67 100644 --- a/app/repositories/report/ReportRepository.scala +++ b/app/repositories/report/ReportRepository.scala @@ -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 @@ -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) => @@ -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, diff --git a/app/repositories/report/ReportRepositoryInterface.scala b/app/repositories/report/ReportRepositoryInterface.scala index 79e7f282..b882bdc1 100644 --- a/app/repositories/report/ReportRepositoryInterface.scala +++ b/app/repositories/report/ReportRepositoryInterface.scala @@ -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, diff --git a/app/repositories/website/WebsiteRepository.scala b/app/repositories/website/WebsiteRepository.scala index d43bed54..c96f942d 100644 --- a/app/repositories/website/WebsiteRepository.scala +++ b/app/repositories/website/WebsiteRepository.scala @@ -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._ @@ -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 @@ -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) diff --git a/app/repositories/website/WebsiteRepositoryInterface.scala b/app/repositories/website/WebsiteRepositoryInterface.scala index dc4ea894..7090e641 100644 --- a/app/repositories/website/WebsiteRepositoryInterface.scala +++ b/app/repositories/website/WebsiteRepositoryInterface.scala @@ -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]] diff --git a/conf/db/migration/default/V38__optimizations.sql b/conf/db/migration/default/V38__optimizations.sql new file mode 100644 index 00000000..155af6cb --- /dev/null +++ b/conf/db/migration/default/V38__optimizations.sql @@ -0,0 +1,2 @@ +create index if not exists idx_auth_attempts_login + on auth_attempts (login); \ No newline at end of file diff --git a/conf/routes b/conf/routes index 69029b64..a3cdffae 100644 --- a/conf/routes +++ b/conf/routes @@ -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() diff --git a/test/controllers/AuthControllerSpec.scala b/test/controllers/AuthControllerSpec.scala index 44484516..a4f8b5e2 100644 --- a/test/controllers/AuthControllerSpec.scala +++ b/test/controllers/AuthControllerSpec.scala @@ -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)) @@ -137,7 +137,7 @@ 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) @@ -145,7 +145,7 @@ class AuthControllerSpec(implicit ee: ExecutionEnv) 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)) @@ -163,7 +163,7 @@ 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) @@ -171,7 +171,7 @@ class AuthControllerSpec(implicit ee: ExecutionEnv) 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)) diff --git a/test/controllers/WebsiteControllerSpec.scala b/test/controllers/WebsiteControllerSpec.scala index 6e28172a..51ff92c6 100644 --- a/test/controllers/WebsiteControllerSpec.scala +++ b/test/controllers/WebsiteControllerSpec.scala @@ -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 diff --git a/test/controllers/report/ReportRepositoryMock.scala b/test/controllers/report/ReportRepositoryMock.scala index ac5090a8..001c6352 100644 --- a/test/controllers/report/ReportRepositoryMock.scala +++ b/test/controllers/report/ReportRepositoryMock.scala @@ -11,7 +11,8 @@ import org.apache.pekko.NotUsed import org.apache.pekko.stream.scaladsl.Source import repositories.report.ReportRepositoryInterface import slick.basic.DatabasePublisher -import utils.{CRUDRepositoryMock, SIRET} +import utils.CRUDRepositoryMock +import utils.SIRET import java.time.LocalDate import java.time.OffsetDateTime @@ -85,7 +86,13 @@ class ReportRepositoryMock(database: mutable.Map[UUID, Report] = mutable.Map.emp override def getPhoneReports(start: Option[LocalDate], end: Option[LocalDate]): Future[List[Report]] = ??? - override 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)]] = ??? + override 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)]] = ??? override def cloudWord(companyId: UUID): Future[List[ReportWordOccurrence]] = ???