Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EPMLSTRCMW-311 Make All Id Types Wrapped #237

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import akka.http.scaladsl.server.{ ExceptionHandler, Route }
import cromwell.pipeline.controller.ProjectConfigurationController._
import cromwell.pipeline.controller.utils.FromStringUnmarshallers._
import cromwell.pipeline.controller.utils.FromUnitMarshaller._
import cromwell.pipeline.controller.utils.PathMatchers.{ Path, ProjectId }
import cromwell.pipeline.controller.utils.PathMatchers.{ Path, ProjectId => ProjectIdPM }
import cromwell.pipeline.datastorage.dto._
import cromwell.pipeline.datastorage.dto.auth.AccessTokenContent
import cromwell.pipeline.model.wrapper.ProjectId
import cromwell.pipeline.service.ProjectConfigurationService
import cromwell.pipeline.service.ProjectConfigurationService.Exceptions._
import de.heikoseeberger.akkahttpplayjson.PlayJsonSupport._
Expand Down Expand Up @@ -61,7 +62,7 @@ class ProjectConfigurationController(projectConfigurationService: ProjectConfigu

val route: AccessTokenContent => Route = implicit accessToken =>
handleExceptions(projectConfigurationServiceExceptionHandler) {
pathPrefix("projects" / ProjectId / "configurations") { projectId =>
pathPrefix("projects" / ProjectIdPM / "configurations") { projectId =>
buildConfiguration(projectId) ~
addConfiguration(projectId) ~
getConfiguration(projectId) ~
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import akka.http.scaladsl.server.Route
import akka.stream.Materializer
import cromwell.pipeline.controller.utils.FieldUnmarshallers._
import cromwell.pipeline.controller.utils.FromStringUnmarshallers._
import cromwell.pipeline.controller.utils.PathMatchers.{ Path, ProjectId }
import cromwell.pipeline.controller.utils.PathMatchers.{ Path, ProjectId => ProjectIdPM }
import cromwell.pipeline.datastorage.dto._
import cromwell.pipeline.datastorage.dto.auth.AccessTokenContent
import cromwell.pipeline.model.wrapper.ProjectId
import cromwell.pipeline.service.ProjectFileService
import de.heikoseeberger.akkahttpplayjson.PlayJsonSupport._

Expand Down Expand Up @@ -99,7 +100,7 @@ class ProjectFileController(wdlService: ProjectFileService)(

val route: AccessTokenContent => Route = implicit accessToken =>
validateFile ~
pathPrefix("projects" / ProjectId / "files") { projectId =>
pathPrefix("projects" / ProjectIdPM / "files") { projectId =>
getFile(projectId) ~
getFiles(projectId) ~
uploadFile(projectId) ~
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ import akka.http.scaladsl.model.{ StatusCode, StatusCodes }
import akka.http.scaladsl.server.Directives.{ entity, _ }
import akka.http.scaladsl.server.{ ExceptionHandler, Route }
import cromwell.pipeline.controller.RunController.runServiceExceptionHandler
import cromwell.pipeline.controller.utils.PathMatchers.{ ProjectId, RunId }
import cromwell.pipeline.controller.utils.PathMatchers.{ RunId, ProjectId => ProjectIdPM }
import cromwell.pipeline.datastorage.dto._
import cromwell.pipeline.datastorage.dto.auth.AccessTokenContent
import cromwell.pipeline.model.wrapper.ProjectId
import cromwell.pipeline.service.RunService
import cromwell.pipeline.service.RunService.Exceptions.RunServiceException
import de.heikoseeberger.akkahttpplayjson.PlayJsonSupport._
Expand Down Expand Up @@ -46,7 +47,7 @@ class RunController(runService: RunService) {

val route: AccessTokenContent => Route = implicit accessToken =>
handleExceptions(runServiceExceptionHandler) {
pathPrefix("projects" / ProjectId / "runs") { projectId =>
pathPrefix("projects" / ProjectIdPM / "runs") { projectId =>
getRun(projectId) ~
getRunsByProject(projectId) ~
deleteRun(projectId) ~
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ package cromwell.pipeline.controller.utils

import akka.http.scaladsl.unmarshalling.Unmarshaller
import cats.data.Validated
import cromwell.pipeline.datastorage.dto.{ PipelineVersion, ProjectId }
import cromwell.pipeline.model.wrapper.RunId
import cromwell.pipeline.datastorage.dto.PipelineVersion
import cromwell.pipeline.model.validator.Enable
import cromwell.pipeline.model.wrapper.{ ProjectId, RunId }

import java.nio.file.{ Path, Paths }

Expand All @@ -17,7 +18,7 @@ object FromStringUnmarshallers {
}
implicit val stringToProjectId: Unmarshaller[String, ProjectId] = Unmarshaller.strict[String, ProjectId] {
projectId =>
ProjectId(projectId)
ProjectId(projectId, Enable.Unsafe)
}
implicit val stringToPath: Unmarshaller[String, Path] = Unmarshaller.strict[String, Path] { path =>
Paths.get(path)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ package cromwell.pipeline.controller.utils

import akka.http.scaladsl.server.PathMatcher1
import akka.http.scaladsl.server.PathMatchers.Segment
import cromwell.pipeline.datastorage.dto
import cromwell.pipeline.model.wrapper

import java.nio.file.{ Path, Paths }

object PathMatchers {
val ProjectId: PathMatcher1[dto.ProjectId] = Segment.map(dto.ProjectId(_))
val ProjectId: PathMatcher1[wrapper.ProjectId] = Segment.flatMap(wrapper.ProjectId.from(_).toOption)

Choose a reason for hiding this comment

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

We should think of generic way implementation to handle all wrappers to PathMatcher, but need to discuss with @myazinn

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. It should relatively be easy to do.

val Path: PathMatcher1[Path] = Segment.map(Paths.get(_))
val RunId: PathMatcher1[wrapper.RunId] = Segment.flatMap(wrapper.RunId.from(_).toOption)
val ProjectSearchFilterId: PathMatcher1[wrapper.ProjectSearchFilterId] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import akka.http.scaladsl.testkit.ScalatestRouteTest
import cromwell.pipeline.datastorage.dao.utils.{ TestProjectUtils, TestUserUtils }
import cromwell.pipeline.datastorage.dto._
import cromwell.pipeline.datastorage.dto.auth.AccessTokenContent
import cromwell.pipeline.model.validator.Enable
import cromwell.pipeline.model.wrapper.ProjectConfigurationId
import cromwell.pipeline.service.ProjectConfigurationService.Exceptions.{ InternalError, NotFound, ValidationError }
import cromwell.pipeline.service.{ ProjectConfigurationService, VersioningException }
import cromwell.pipeline.utils.URLEncoderUtils
Expand All @@ -14,6 +16,7 @@ import org.scalatest.{ AsyncWordSpec, Matchers }
import org.scalatestplus.mockito.MockitoSugar

import java.nio.file.Paths
import java.util.UUID
import scala.concurrent.Future

class ProjectConfigurationControllerTest extends AsyncWordSpec with Matchers with ScalatestRouteTest with MockitoSugar {
Expand All @@ -24,7 +27,7 @@ class ProjectConfigurationControllerTest extends AsyncWordSpec with Matchers wit
val accessToken = AccessTokenContent(TestUserUtils.getDummyUserId)
val projectId = TestProjectUtils.getDummyProjectId
val configuration = ProjectConfiguration(
ProjectConfigurationId.randomId,
ProjectConfigurationId(UUID.randomUUID().toString, Enable.Unsafe),
projectId,
active = true,
WdlParams(Paths.get("/home/file"), List(FileParameter("nodeName", StringTyped(Some("hello"))))),
Expand All @@ -46,21 +49,19 @@ class ProjectConfigurationControllerTest extends AsyncWordSpec with Matchers wit

"return success for update configuration" in {
when(configurationService.addConfiguration(configuration, accessToken.userId)).thenReturn(Future.unit)
Put(s"/projects/${projectId.value}/configurations", configurationAdditionRequest) ~> configurationController
.route(
accessToken
) ~> check {
Put(s"/projects/$projectId/configurations", configurationAdditionRequest) ~> configurationController.route(
accessToken
) ~> check {
status shouldBe StatusCodes.NoContent
}
}

"return InternalServerError when failure update configuration" in {
val error = InternalError("Something went wrong")
when(configurationService.addConfiguration(configuration, accessToken.userId)).thenReturn(Future.failed(error))
Put(s"/projects/${projectId.value}/configurations", configurationAdditionRequest) ~> configurationController
.route(
accessToken
) ~> check {
Put(s"/projects/$projectId/configurations", configurationAdditionRequest) ~> configurationController.route(
accessToken
) ~> check {
status shouldBe StatusCodes.InternalServerError
entityAs[String] shouldBe "Something went wrong"
}
Expand All @@ -69,10 +70,9 @@ class ProjectConfigurationControllerTest extends AsyncWordSpec with Matchers wit
"return NotFound when failure find project to update configuration" in {
when(configurationService.addConfiguration(configuration, accessToken.userId))
.thenReturn(Future.failed(NotFound()))
Put(s"/projects/${projectId.value}/configurations", configurationAdditionRequest) ~> configurationController
.route(
accessToken
) ~> check {
Put(s"/projects/$projectId/configurations", configurationAdditionRequest) ~> configurationController.route(
accessToken
) ~> check {
status shouldBe StatusCodes.NotFound
}
}
Expand All @@ -82,18 +82,18 @@ class ProjectConfigurationControllerTest extends AsyncWordSpec with Matchers wit
"return configuration by existing project id" in {
when(configurationService.getLastByProjectId(projectId, accessToken.userId))
.thenReturn(Future.successful(configuration))
Get(s"/projects/${projectId.value}/configurations") ~> configurationController.route(accessToken) ~> check {
Get(s"/projects/$projectId/configurations") ~> configurationController.route(accessToken) ~> check {
status shouldBe StatusCodes.OK
entityAs[ProjectConfiguration] shouldBe configuration
}
}

"return Configuration not found message" in {
when(configurationService.getLastByProjectId(projectId, accessToken.userId))
.thenReturn(Future.failed(NotFound(s"There is no configuration with project_id: ${projectId.value}")))
Get(s"/projects/${projectId.value}/configurations") ~> configurationController.route(accessToken) ~> check {
.thenReturn(Future.failed(NotFound(s"There is no configuration with project_id: $projectId")))
Get(s"/projects/$projectId/configurations") ~> configurationController.route(accessToken) ~> check {
status shouldBe StatusCodes.NotFound
entityAs[String] shouldBe s"There is no configuration with project_id: ${projectId.value}"
entityAs[String] shouldBe s"There is no configuration with project_id: $projectId"
}
}
}
Expand All @@ -103,15 +103,15 @@ class ProjectConfigurationControllerTest extends AsyncWordSpec with Matchers wit

"return success for deactivate configuration" in {
when(configurationService.deactivateLastByProjectId(projectId, accessToken.userId)).thenReturn(Future.unit)
Delete(s"/projects/${projectId.value}/configurations") ~> configurationController.route(accessToken) ~> check {
Delete(s"/projects/$projectId/configurations") ~> configurationController.route(accessToken) ~> check {
status shouldBe StatusCodes.NoContent
}
}

"return InternalServerError when failure deactivate configuration" in {
when(configurationService.deactivateLastByProjectId(projectId, accessToken.userId))
.thenReturn(Future.failed(error))
Delete(s"/projects/${projectId.value}/configurations") ~> configurationController.route(accessToken) ~> check {
Delete(s"/projects/$projectId/configurations") ~> configurationController.route(accessToken) ~> check {
status shouldBe StatusCodes.InternalServerError
entityAs[String] shouldBe "Something went wrong"
}
Expand All @@ -120,7 +120,7 @@ class ProjectConfigurationControllerTest extends AsyncWordSpec with Matchers wit
"return NotFound when failure find project to deactivate configuration" in {
when(configurationService.deactivateLastByProjectId(projectId, accessToken.userId))
.thenReturn(Future.failed(NotFound()))
Delete(s"/projects/${projectId.value}/configurations") ~> configurationController.route(accessToken) ~> check {
Delete(s"/projects/$projectId/configurations") ~> configurationController.route(accessToken) ~> check {
status shouldBe StatusCodes.NotFound
}
}
Expand All @@ -131,7 +131,7 @@ class ProjectConfigurationControllerTest extends AsyncWordSpec with Matchers wit
"return configuration for file" in {
when(configurationService.buildConfiguration(projectId, path, versionOption, accessToken.userId))
.thenReturn(Future.successful(configuration))
Get(s"/projects/${projectId.value}/configurations/files/$pathString?version=$versionString") ~>
Get(s"/projects/$projectId/configurations/files/$pathString?version=$versionString") ~>
configurationController.route(accessToken) ~> check {
status shouldBe StatusCodes.OK
entityAs[ProjectConfiguration] shouldBe configuration
Expand All @@ -141,7 +141,7 @@ class ProjectConfigurationControllerTest extends AsyncWordSpec with Matchers wit
"return failed for Bad request" in {
when(configurationService.buildConfiguration(projectId, path, versionOption, accessToken.userId))
.thenReturn(Future.failed(VersioningException.HttpException("Bad request")))
Get(s"/projects/${projectId.value}/configurations/files/$pathString?version=$versionString") ~>
Get(s"/projects/$projectId/configurations/files/$pathString?version=$versionString") ~>
configurationController.route(accessToken) ~> check {
status shouldBe StatusCodes.InternalServerError
entityAs[String] shouldBe "Bad request"
Expand All @@ -151,7 +151,7 @@ class ProjectConfigurationControllerTest extends AsyncWordSpec with Matchers wit
"return failed for invalid file" in {
when(configurationService.buildConfiguration(projectId, path, versionOption, accessToken.userId))
.thenReturn(Future.failed(ValidationError(List("invalid some field").mkString(","))))
Get(s"/projects/${projectId.value}/configurations/files/$pathString?version=$versionString") ~>
Get(s"/projects/$projectId/configurations/files/$pathString?version=$versionString") ~>
configurationController.route(accessToken) ~> check {
status shouldBe StatusCodes.UnprocessableEntity
entityAs[String] shouldBe "invalid some field"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class ProjectControllerTest extends AsyncWordSpec with Matchers with ScalatestRo
when(projectService.getUserProjectById(dummyProject.projectId, accessToken.userId))
.thenReturn(Future.successful(dummyProject))

Get(s"/projects/${dummyProject.projectId.value}") ~> projectController.route(accessToken) ~> check {
Get(s"/projects/${dummyProject.projectId}") ~> projectController.route(accessToken) ~> check {
status shouldBe StatusCodes.OK
responseAs[Project] shouldEqual dummyProject
}
Expand All @@ -72,7 +72,7 @@ class ProjectControllerTest extends AsyncWordSpec with Matchers with ScalatestRo
when(projectService.getUserProjectById(dummyProject.projectId, accessToken.userId))
.thenReturn(Future.failed(error))

Get(s"/projects/${dummyProject.projectId.value}") ~> projectController.route(accessToken) ~> check {
Get(s"/projects/${dummyProject.projectId}") ~> projectController.route(accessToken) ~> check {
status shouldBe StatusCodes.InternalServerError
}
}
Expand Down Expand Up @@ -131,7 +131,7 @@ class ProjectControllerTest extends AsyncWordSpec with Matchers with ScalatestRo
projectService.deactivateProjectById(dummyProject.projectId, accessToken.userId)
).thenReturn(Future.successful(deactivatedProject))

Delete(s"/projects/${dummyProject.projectId.value}") ~> projectController.route(accessToken) ~> check {
Delete(s"/projects/${dummyProject.projectId}") ~> projectController.route(accessToken) ~> check {
responseAs[Project] shouldBe deactivatedProject
}
}
Expand All @@ -142,7 +142,7 @@ class ProjectControllerTest extends AsyncWordSpec with Matchers with ScalatestRo
when(projectService.deactivateProjectById(projectId, strangerAccessToken.userId))
.thenReturn(Future.failed(InternalError()))

Delete(s"/projects/${projectId.value}") ~> projectController.route(strangerAccessToken) ~> check {
Delete(s"/projects/${projectId}") ~> projectController.route(strangerAccessToken) ~> check {
status shouldBe StatusCodes.InternalServerError
}
}
Expand All @@ -155,7 +155,7 @@ class ProjectControllerTest extends AsyncWordSpec with Matchers with ScalatestRo
when(projectService.updateProjectName(dummyProject.projectId, request, accessToken.userId))
.thenReturn(Future.successful(dummyProject.projectId))

Put(s"/projects/${dummyProject.projectId.value}", request) ~> projectController.route(accessToken) ~> check {
Put(s"/projects/${dummyProject.projectId}", request) ~> projectController.route(accessToken) ~> check {
status shouldBe StatusCodes.OK
}
}
Expand All @@ -166,7 +166,7 @@ class ProjectControllerTest extends AsyncWordSpec with Matchers with ScalatestRo
when(projectService.updateProjectName(dummyProject.projectId, request, strangerAccessToken.userId))
.thenReturn(Future.failed(InternalError()))

Put(s"/projects/${dummyProject.projectId.value}", request) ~> projectController.route(strangerAccessToken) ~> check {
Put(s"/projects/${dummyProject.projectId}", request) ~> projectController.route(strangerAccessToken) ~> check {
status shouldBe StatusCodes.InternalServerError
}
}
Expand Down
Loading