Refactors some of the logic around frontend logging endpoint

augustinas · augustinas · commit 1345be0d6a05 · 2019-02-25T19:23:17.000Z
diff --git a/hat/app/org/hatdex/hat/api/controllers/Authentication.scala b/hat/app/org/hatdex/hat/api/controllers/Authentication.scala
@@ -26,6 +26,7 @@ package org.hatdex.hat.api.controllers
 
 import java.net.{ URLDecoder, URLEncoder }
 
+import akka.Done
 import javax.inject.Inject
 import com.mohiva.play.silhouette.api.repositories.AuthInfoRepository
 import com.mohiva.play.silhouette.api.util.{ Credentials, PasswordHasherRegistry }
@@ -246,9 +247,10 @@ class Authentication @Inject() (
         case Some(user) =>
           applicationsService.applicationStatus()(request.dynamicEnvironment, user, request).flatMap { applications =>
             val maybeApplication = applications.find(_.application.id.equals(claimHatRequest.applicationId))
-            val maybeEmailDetails = maybeApplication.map(app => {
-              (app.application.info.name, app.application.info.graphics.logo.normal)
-            })
+            val maybeAppDetails = maybeApplication.map { app =>
+              ((app.application.info.name, app.application.developer.logo.map(_.normal).getOrElse("#")),
+                (app.application.id, app.application.info.version.toString))
+            }
 
             val scheme = if (request.secure) {
               "https://"
@@ -260,23 +262,36 @@ class Authentication @Inject() (
             tokenService.retrieve(email, isSignup = true).flatMap {
               case Some(existingTokenUser) if !existingTokenUser.isExpired =>
                 val claimLink = s"$scheme${request.host}/#/hat/claim/${existingTokenUser.id}?email=${URLEncoder.encode(email, "UTF-8")}"
-                mailer.claimHat(email, claimLink, maybeEmailDetails)
+                mailer.claimHat(email, claimLink, maybeAppDetails.map(_._1))
 
                 Future.successful(response)
               case Some(_) => Future.successful(Ok(Json.toJson(SuccessResponse("The HAT is already claimed"))))
 
               case None =>
-                logService.logAction(request.dynamicEnvironment.domain, "unclaimed", None, None, Some((claimHatRequest.applicationId, applicationVersion))) recover {
-                  case _ => logger.error("LogActionError::unclaimed")
-                }
-
                 val token = MailClaimTokenUser(email)
-                tokenService.create(token).map { _ =>
+
+                val eventualResult = for {
+                  _ <- tokenService.create(token)
+                  _ <- logService
+                    .logAction(request.dynamicEnvironment.domain, LogRequest("unclaimed", None, None), maybeAppDetails.map(_._2))
+                    .recover {
+                      case e =>
+                        logger.error(s"LogActionError::unclaimed. Reason: ${e.getMessage}")
+                        Done
+                    }
+                } yield {
+
                   val claimLink = s"$scheme${request.host}/#/hat/claim/${token.id}?email=${URLEncoder.encode(email, "UTF-8")}"
-                  mailer.claimHat(email, claimLink, maybeEmailDetails)
+                  mailer.claimHat(email, claimLink, maybeAppDetails.map(_._1))
 
                   response
                 }
+
+                eventualResult.recover {
+                  case e =>
+                    logger.error(s"Could not create new HAT claim token. Reason: ${e.getMessage}")
+                    InternalServerError(Json.toJson(ErrorMessage("Internal Server Error", "Failed to initialize HAT claim process")))
+                }
             }
           }
         case None => Future.successful(response)
diff --git a/hat/app/org/hatdex/hat/api/controllers/HattersRequestProxy.scala b/hat/app/org/hatdex/hat/api/controllers/HattersRequestProxy.scala
@@ -48,23 +48,30 @@ class HattersRequestProxy @Inject() (
       case Some(token) if token.isSignUp && !token.isExpired && token.email == request.dynamicEnvironment.ownerEmail =>
         usersService.listUsers.map(_.find(_.roles.contains(Owner()))).flatMap {
           case Some(user) =>
-            for {
+            val eventualResult = for {
               _ <- updateHatMembership(hatClaimComplete)
               _ <- authInfoRepository.update(user.loginInfo, passwordHasherRegistry.current.hash(request.body.password))
               _ <- tokenService.expire(token.id)
               authenticator <- env.authenticatorService.create(user.loginInfo)
               result <- env.authenticatorService.renew(authenticator, Ok(Json.toJson(SuccessResponse("HAT claimed"))))
+              _ <- logService.logAction(request.dynamicEnvironment.domain, LogRequest("claimed", None, None), None).recover {
+                case e =>
+                  logger.error(s"LogActionError::unclaimed. Reason: ${e.getMessage}")
+                  Done
+              }
             } yield {
               //env.eventBus.publish(LoginEvent(user, request))
               //mailer.passwordChanged(token.email, user)
 
-              logService.logAction(request.dynamicEnvironment.domain, "claimed", None, None, None).recover {
-                case _ => logger.error("LogActionError::claimed")
-              }
-
               result
             }
 
+            eventualResult.recover {
+              case e =>
+                logger.error(s"HAT claim process failed with error ${e.getMessage}")
+                BadRequest(Json.toJson(ErrorMessage("Bad Request", "HAT claim process failed")))
+            }
+
           case None => Future.successful(Unauthorized(Json.toJson(ErrorMessage("HAT claim unauthorized", "No user matching token"))))
         }
 
diff --git a/hat/app/org/hatdex/hat/api/controllers/LogController.scala b/hat/app/org/hatdex/hat/api/controllers/LogController.scala
@@ -2,45 +2,39 @@ package org.hatdex.hat.api.controllers
 
 import com.mohiva.play.silhouette.api.Silhouette
 import javax.inject.Inject
-import org.hatdex.hat.api.json.ApplicationJsonProtocol
+import org.hatdex.hat.api.json.HatJsonFormats
 import org.hatdex.hat.api.models._
 import org.hatdex.hat.api.service.{ LogService, RemoteExecutionContext }
-<<<<<<< HEAD
-import org.hatdex.hat.authentication.{ ContainsApplicationRole, HatApiAuthEnvironment, HatApiController, WithRole }
-=======
 import org.hatdex.hat.authentication.{ HatApiAuthEnvironment, HatApiController }
->>>>>>> b8d0df1... Completed ActionLog fixes
 import org.hatdex.hat.utils.HatBodyParsers
-import org.hatdex.hat.api.json.HatJsonFormats._
 import play.api.Logger
 import play.api.libs.json.Json
 import play.api.mvc.{ Action, ControllerComponents }
 
+import scala.util.Try
+
 class LogController @Inject() (
     components: ControllerComponents,
     parsers: HatBodyParsers,
-    silhouette: Silhouette[HatApiAuthEnvironment])(
-    implicit
-    val ec: RemoteExecutionContext,
-    logService: LogService)
-  extends HatApiController(components, silhouette) with ApplicationJsonProtocol {
+    logService: LogService,
+    silhouette: Silhouette[HatApiAuthEnvironment])(implicit val ec: RemoteExecutionContext)
+  extends HatApiController(components, silhouette) with HatJsonFormats {
 
-  protected val logger = Logger(this.getClass)
+  private val logger = Logger(this.getClass)
 
-  def log(): Action[LogRequest] = SecuredAction.async(parsers.json[LogRequest]) { request =>
+  def logFrontendAction(): Action[LogRequest] = SecuredAction.async(parsers.json[LogRequest]) { request =>
     val logRequest = request.body
     val hatAddress = request.dynamicEnvironment.domain
-    val (applicationId, applicationVersion) = request.authenticator.customClaims.map { customClaims =>
-      ((customClaims \ "application").asOpt[String], (customClaims \ "applicationVersion").asOpt[String])
-    }.getOrElse((None, None))
+    val appDetails = request.authenticator.customClaims.flatMap { customClaims =>
+      Try(((customClaims \ "application").as[String], (customClaims \ "applicationVersion").as[String])).toOption
+    }
 
-    logService.logAction(hatAddress, logRequest.actionCode, logRequest.message, logRequest.logGroup, applicationData).map { _ =>
+    logService.logAction(hatAddress, logRequest, appDetails).map { _ =>
       Ok(Json.toJson(SuccessResponse(s"${logRequest.actionCode} logged")))
     }.recover {
-      case _ => {
-        logger.error(s"LogActionError::${logRequest.actionCode}")
-        BadRequest(Json.toJson(ErrorMessage("LogActionError", s"LogActionError::${logRequest.actionCode} logging failed")))
-      }
+      case e =>
+        logger.error(s"Failed to log action ${logRequest.actionCode}. Reason:\n${e.getMessage}")
+        InternalServerError(Json.toJson(ErrorMessage("Internal server error", s"Could not log ${logRequest.actionCode} action")))
     }
   }
 }
diff --git a/hat/app/org/hatdex/hat/api/models/LogRequest.scala b/hat/app/org/hatdex/hat/api/models/LogRequest.scala
diff --git a/hat/app/org/hatdex/hat/api/service/LogService.scala b/hat/app/org/hatdex/hat/api/service/LogService.scala
@@ -26,19 +26,23 @@ package org.hatdex.hat.api.service
 
 import java.util.UUID
 
+import akka.Done
 import javax.inject.Inject
+import org.hatdex.hat.api.models.LogRequest
 import play.api.Logger
 
 import scala.concurrent.Future
 
 class LogService @Inject() (implicit val ec: DalExecutionContext) {
   val logger = Logger(this.getClass)
 
-  def logAction(hat: String, actionCode: String, message: Option[String], logGroup: Option[String], applicationData: Option[(String, String)]): Future[Unit] = {
+  def logAction(hat: String, logDetails: LogRequest, applicationDetails: Option[(String, String)]): Future[Done] = {
     Future {
       val logId = UUID.randomUUID()
-      val appVersion = applicationData.map(app => s"${app._1}@${app._2}")
-      logger.info(s"[${logGroup.getOrElse("STATS")}] [$hat] [$logId] [$actionCode] [${message.getOrElse("")}] ${appVersion.getOrElse("_")}")
+      val applicationVersion = applicationDetails.map(a => s"${a._1}@${a._2}").getOrElse("Unknown")
+      logger.info(s"[${logDetails.logGroup.getOrElse("STATS")}] [$hat] [$logId] [${logDetails.actionCode}] [$applicationVersion] ${logDetails.message.getOrElse("")}")
+
+      Done
     }
   }
 }
diff --git a/hat/conf/routes b/hat/conf/routes
@@ -28,9 +28,6 @@ POST        /control/v2/auth/passwordreset/confirm/:token        org.hatdex.hat.
 POST        /control/v2/auth/claim                               org.hatdex.hat.api.controllers.Authentication.claim
 POST        /control/v2/auth/claim/complete/:claimToken          org.hatdex.hat.api.controllers.HattersRequestProxy.proxyRequestHatClaim(claimToken: String)
 
-# LOG routes
-POST        /log                                                 org.hatdex.hat.api.controllers.LogController.log
-
 ->          /api/v2                                              v20.Routes
 ->          /api/v2.0                                            v20.Routes
 ->          /api/v2.6                                            v26.Routes
diff --git a/hat/conf/v26.routes b/hat/conf/v26.routes
@@ -1,6 +1,7 @@
 
 GET           /system/status                                                                 org.hatdex.hat.api.controllers.SystemStatus.status()
 GET           /phata/profile                                                                 org.hatdex.hat.phata.controllers.Phata.profile
+POST          /report-frontend-action                                                        org.hatdex.hat.api.controllers.LogController.logFrontendAction
 
 # FILE routes
 POST          /files/upload                                                                  org.hatdex.hat.api.controllers.Files.startUpload
diff --git a/project/Dependencies.scala b/project/Dependencies.scala
@@ -93,7 +93,7 @@ object Dependencies {
     }
 
     object HATDeX {
-      private val version = "2.6.2-SNAPSHOT"
+      private val version = "2.6.5-SNAPSHOT"
       val hatClient = "org.hatdex" %% "hat-client-scala-play" % version
       val dexClient = "org.hatdex" %% "dex-client-scala-play" % "2.6.4-SNAPSHOT"
       val codegen = "org.hatdex" %% "slick-postgres-driver" % "0.0.9"

Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,7 @@ object Dependencies {`
`93`	`93`	`}`
`94`	`94`
`95`	`95`	`object HATDeX {`
`96`		`- private val version = "2.6.2-SNAPSHOT"`
	`96`	`+ private val version = "2.6.5-SNAPSHOT"`
`97`	`97`	`val hatClient = "org.hatdex" %% "hat-client-scala-play" % version`
`98`	`98`	`val dexClient = "org.hatdex" %% "dex-client-scala-play" % "2.6.4-SNAPSHOT"`
`99`	`99`	`val codegen = "org.hatdex" %% "slick-postgres-driver" % "0.0.9"`