Skip to content

Commit

Permalink
Allow ID wrappers as path variables
Browse files Browse the repository at this point in the history
Previously, if we wanted to include an ID value in the URL path, we had to declare
the path variable as `Long` in the controller and then explicitly create an ID
wrapper object.

This was needed because Spring MVC didn't know how to convert a string to an
instance of an ID wrapper class. So the simple fix is to add a constructor to the
wrapper classes that takes a string argument. If the client uses a non-numeric
value, it will fail with HTTP 400 the same way it fails with `Long` parameters.

Update all the controller methods that used to take `Long` arguments to instead
use the appropriate wrapper classes.

This caused us to hit swagger-api/swagger-core#3904 so
upgrade the OpenAPI library to get the fix for that.
  • Loading branch information
sgrimm committed Sep 2, 2021
1 parent 5f6d5b9 commit bff8811
Show file tree
Hide file tree
Showing 18 changed files with 143 additions and 161 deletions.
2 changes: 1 addition & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ dependencies {
implementation("com.fasterxml.jackson:jackson-bom:$jacksonVersion")
implementation("com.fasterxml.jackson.module:jackson-module-kotlin:$jacksonVersion")
implementation("com.opencsv:opencsv:5.3")
implementation("io.swagger.core.v3:swagger-annotations:2.1.7")
implementation("io.swagger.core.v3:swagger-annotations:2.1.10")
implementation("javax.inject:javax.inject:1")
implementation("net.postgis:postgis-jdbc:2021.1.0")
implementation("net.rakugakibox.spring.boot:logback-access-spring-boot-starter:2.7.1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class IdWrapper(
fun render(out: JavaWriter) {
out.println("""
class $className @JsonCreator constructor(@get:JsonValue val value: Long) {
constructor(value: String) : this(value.toLong())
override fun equals(other: Any?): Boolean = other is $className && other.value == value
override fun hashCode(): Int = value.hashCode()
override fun toString(): String = value.toString()
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ jacksonVersion=2.11.3
jooqVersion=3.14.7
kotlinVersion=1.5.10
postgresJdbcVersion=42.2.19
springDocVersion=1.5.5
springDocVersion=1.5.10
3 changes: 3 additions & 0 deletions openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,9 @@ paths:
content:
multipart/form-data:
schema:
required:
- file
- metadata
type: object
properties:
file:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ class AdminController(
}

@GetMapping("/organization/{organizationId}")
fun organization(@PathVariable("organizationId") idValue: Long, model: Model): String {
val organizationId = OrganizationId(idValue)
fun organization(@PathVariable organizationId: OrganizationId, model: Model): String {
val organization = organizationStore.fetchById(organizationId) ?: throw NotFoundException()
val projects = projectStore.fetchByOrganization(organizationId).sortedBy { it.name }
val users = organizationStore.fetchUsers(listOf(organizationId)).sortedBy { it.email }
Expand Down Expand Up @@ -101,8 +100,7 @@ class AdminController(
}

@GetMapping("/project/{projectId}")
fun project(@PathVariable("projectId") idValue: Long, model: Model): String {
val projectId = ProjectId(idValue)
fun project(@PathVariable projectId: ProjectId, model: Model): String {
val project = projectStore.fetchById(projectId) ?: throw NotFoundException()
val organization = organizationStore.fetchById(project.organizationId)
val orgUsers =
Expand All @@ -123,8 +121,7 @@ class AdminController(
}

@GetMapping("/site/{siteId}")
fun site(@PathVariable("siteId") idValue: Long, model: Model): String {
val siteId = SiteId(idValue)
fun site(@PathVariable siteId: SiteId, model: Model): String {
val site = siteStore.fetchById(siteId) ?: throw NotFoundException()
val projectId = site.projectId
val project = projectStore.fetchById(projectId) ?: throw NotFoundException()
Expand All @@ -143,8 +140,7 @@ class AdminController(
}

@GetMapping("/user/{userId}")
fun user(@PathVariable("userId") idValue: Long, model: Model): String {
val userId = UserId(idValue)
fun user(@PathVariable userId: UserId, model: Model): String {
val user = userStore.fetchById(userId)
val organizations = organizationStore.fetchAll() // TODO: Only ones where currentUser is admin?
val projects = projectStore.fetchAll().groupBy { it.organizationId }
Expand All @@ -160,15 +156,14 @@ class AdminController(

@PostMapping("/setUserMemberships")
fun setUserMemberships(
@RequestParam("userId") userIdValue: Long,
@RequestParam("organizationId", required = false) organizationIdValues: List<Long>?,
@RequestParam("userId") userId: UserId,
@RequestParam("organizationId", required = false) organizationIdList: List<OrganizationId>?,
@RequestParam("role", required = false) roleValues: List<String>?,
@RequestParam("projectId", required = false) projectIdValues: List<Long>?,
@RequestParam("projectId", required = false) projectIdList: List<ProjectId>?,
model: Model
): String {
val organizationIds = organizationIdValues?.map { OrganizationId(it) }?.toSet() ?: emptySet()
val projectIds = projectIdValues?.map { ProjectId(it) }?.toSet() ?: emptySet()
val userId = UserId(userIdValue)
val organizationIds = organizationIdList?.toSet() ?: emptySet()
val projectIds = projectIdList?.toSet() ?: emptySet()

// Roles are of the form orgId:roleId
val roles =
Expand Down Expand Up @@ -228,11 +223,11 @@ class AdminController(

model.addAttribute("successMessage", "User memberships updated.")

return user(userIdValue, model)
return user(userId, model)
}

@PostMapping("/createOrganization")
fun createOrganization(@NotBlank @RequestParam name: String, model: Model): String {
fun createOrganization(@NotBlank @RequestParam("name") name: String, model: Model): String {
try {
val org = organizationStore.createWithAdmin(name)
model.addAttribute("successMessage", "Created organization ${org.id}")
Expand All @@ -246,23 +241,22 @@ class AdminController(

@PostMapping("/addOrganizationUser")
fun addOrganizationUser(
@RequestParam("organizationId") organizationIdValue: Long,
@RequestParam("organizationId") organizationId: OrganizationId,
@NotBlank @RequestParam("email") email: String,
@RequestParam("role") roleId: Int,
model: Model
): String {
val organizationId = OrganizationId(organizationIdValue)
val role = Role.of(roleId)

if (role == null) {
model.addAttribute("failureMessage", "Invalid role selected.")
return organization(organizationIdValue, model)
return organization(organizationId, model)
}

val userDetails = userStore.fetchByEmail(email)
if (userDetails == null) {
model.addAttribute("failureMessage", "User $email not found.")
return organization(organizationIdValue, model)
return organization(organizationId, model)
}

try {
Expand All @@ -274,18 +268,15 @@ class AdminController(
model.addAttribute("failureMessage", "$email was already a member of the organization.")
}

return organization(organizationIdValue, model)
return organization(organizationId, model)
}

@PostMapping("/removeOrganizationUser")
fun removeOrganizationUser(
@RequestParam("organizationId") organizationIdValue: Long,
@RequestParam("userId") userIdValue: Long,
@RequestParam("organizationId") organizationId: OrganizationId,
@RequestParam("userId") userId: UserId,
model: Model
): String {
val organizationId = OrganizationId(organizationIdValue)
val userId = UserId(userIdValue)

try {
if (organizationStore.removeUser(organizationId, userId)) {
model.addAttribute("successMessage", "User removed from organization.")
Expand All @@ -296,23 +287,21 @@ class AdminController(
model.addAttribute("failureMessage", "No permission to remove users from this organization.")
}

return organization(organizationIdValue, model)
return organization(organizationId, model)
}

@PostMapping("/setOrganizationUserRole")
fun setOrganizationUserRole(
@RequestParam("organizationId") organizationIdValue: Long,
@RequestParam("userId") userIdValue: Long,
@RequestParam("organizationId") organizationId: OrganizationId,
@RequestParam("userId") userId: UserId,
@RequestParam("role") roleId: Int,
model: Model
): String {
val organizationId = OrganizationId(organizationIdValue)
val role = Role.of(roleId)
val userId = UserId(userIdValue)

if (role == null) {
model.addAttribute("failureMessage", "Invalid role selected.")
return organization(organizationIdValue, model)
return organization(organizationId, model)
}

try {
Expand All @@ -325,36 +314,31 @@ class AdminController(
model.addAttribute("failureMessage", "No permission to set user roles for this organization.")
}

return organization(organizationIdValue, model)
return organization(organizationId, model)
}

@PostMapping("/createProject")
fun createProject(
@RequestParam("organizationId") organizationIdValue: Long,
@RequestParam("organizationId") organizationId: OrganizationId,
@NotBlank @RequestParam("name") name: String,
model: Model
): String {
val organizationId = OrganizationId(organizationIdValue)

try {
projectStore.create(organizationId, name)
model.addAttribute("successMessage", "Project created.")
} catch (e: AccessDeniedException) {
model.addAttribute("failureMessage", "No permission to create projects in this organization.")
}

return organization(organizationIdValue, model)
return organization(organizationId, model)
}

@PostMapping("/removeProjectUser")
fun removeProjectUser(
@RequestParam("projectId") projectIdValue: Long,
@RequestParam("userId") userIdValue: Long,
@RequestParam("projectId") projectId: ProjectId,
@RequestParam("userId") userId: UserId,
model: Model
): String {
val projectId = ProjectId(projectIdValue)
val userId = UserId(userIdValue)

try {
if (projectStore.removeUser(projectId, userId)) {
model.addAttribute("successMessage", "User removed from project.")
Expand All @@ -365,18 +349,15 @@ class AdminController(
model.addAttribute("failureMessage", "No permission to remove users from this project.")
}

return project(projectIdValue, model)
return project(projectId, model)
}

@PostMapping("/addProjectUser")
fun addProjectUser(
@RequestParam("projectId") projectIdValue: Long,
@RequestParam("userId") userIdValue: Long,
@RequestParam("projectId") projectId: ProjectId,
@RequestParam("userId") userId: UserId,
model: Model
): String {
val projectId = ProjectId(projectIdValue)
val userId = UserId(userIdValue)

try {
projectStore.addUser(projectId, userId)
model.addAttribute("successMessage", "User added to project.")
Expand All @@ -386,53 +367,49 @@ class AdminController(
model.addAttribute("failureMessage", "User is already in this project.")
}

return project(projectIdValue, model)
return project(projectId, model)
}

@PostMapping("/createSite")
fun createSite(
@RequestParam("projectId") projectIdValue: Long,
@RequestParam("projectId") projectId: ProjectId,
@NotBlank @RequestParam("name") name: String,
@Min(-180L) @Max(180L) @RequestParam("latitude") latitude: BigDecimal,
@Min(-180L) @Max(180L) @RequestParam("longitude") longitude: BigDecimal,
model: Model
): String {
val projectId = ProjectId(projectIdValue)

siteStore.create(projectId, name, latitude, longitude)

model.addAttribute("successMessage", "Site created.")
return project(projectIdValue, model)
return project(projectId, model)
}

@PostMapping("/createFacility")
fun createFacility(
@RequestParam("siteId") siteIdValue: Long,
@RequestParam("siteId") siteId: SiteId,
@NotBlank @RequestParam("name") name: String,
@RequestParam("type") typeId: Int,
model: Model
): String {
val siteId = SiteId(siteIdValue)
val type = FacilityType.forId(typeId)

if (type == null) {
model.addAttribute("failureMessage", "Unknown facility type.")
return site(siteIdValue, model)
return site(siteId, model)
}

facilityStore.create(siteId, name, type)

model.addAttribute("successMessage", "Facility created.")
return site(siteIdValue, model)
return site(siteId, model)
}

@PostMapping("/createApiKey")
fun createApiKey(
@RequestParam("organizationId") organizationIdValue: Long,
@RequestParam("organizationId") organizationId: OrganizationId,
@RequestParam("description") description: String?,
model: Model
): String {
val organizationId = OrganizationId(organizationIdValue)
val organization = organizationStore.fetchById(organizationId)
val newUser = userStore.createApiClient(organizationId, description)

Expand All @@ -450,18 +427,16 @@ class AdminController(

@PostMapping("/deleteApiKey")
fun deleteApiKey(
@RequestParam("organizationId") organizationIdValue: Long,
@RequestParam("userId") userIdValue: Long,
@RequestParam("organizationId") organizationId: OrganizationId,
@RequestParam("userId") userId: UserId,
model: Model
): String {
val userId = UserId(userIdValue)

if (userStore.deleteApiClient(userId)) {
model.addAttribute("successMessage", "API key deleted.")
} else {
model.addAttribute("failureMessage", "Unable to delete API key.")
}

return organization(organizationIdValue, model)
return organization(organizationId, model)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.terraformation.backend.customer.api
import com.terraformation.backend.api.CustomerEndpoint
import com.terraformation.backend.auth.currentUser
import com.terraformation.backend.customer.db.FacilityStore
import com.terraformation.backend.db.FacilityId
import io.swagger.v3.oas.annotations.Operation
import org.springframework.web.bind.annotation.GetMapping
import org.springframework.web.bind.annotation.RequestMapping
Expand All @@ -28,13 +29,18 @@ class FacilityController(private val facilityStore: FacilityStore) {
facilityRoles[facility.id]
?: throw IllegalStateException(
"BUG! No role for facility that was selected based on the presence of a role")
ListFacilitiesElement(facility.id.value, facility.name, facility.type.id, role.name)
ListFacilitiesElement(facility.id, facility.name, facility.type.id, role.name)
}

return ListFacilitiesResponse(elements)
}
}

data class ListFacilitiesElement(val id: Long, val name: String, val type: Int, val role: String)
data class ListFacilitiesElement(
val id: FacilityId,
val name: String,
val type: Int,
val role: String
)

data class ListFacilitiesResponse(val facilities: List<ListFacilitiesElement>)
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.terraformation.backend.customer.api

import com.terraformation.backend.api.CustomerEndpoint
import com.terraformation.backend.customer.db.OrganizationStore
import com.terraformation.backend.db.OrganizationId
import io.swagger.v3.oas.annotations.Operation
import org.springframework.web.bind.annotation.GetMapping
import org.springframework.web.bind.annotation.RequestMapping
Expand All @@ -18,13 +19,11 @@ class OrganizationController(private val organizationStore: OrganizationStore) {
)
fun listAll(): ListOrganizationsResponse {
val elements =
organizationStore.fetchAll().map { model ->
ListOrganizationsElement(model.id.value, model.name)
}
organizationStore.fetchAll().map { model -> ListOrganizationsElement(model.id, model.name) }
return ListOrganizationsResponse(elements)
}
}

data class ListOrganizationsElement(val id: Long, val name: String)
data class ListOrganizationsElement(val id: OrganizationId, val name: String)

data class ListOrganizationsResponse(val organizations: List<ListOrganizationsElement>)
Loading

0 comments on commit bff8811

Please sign in to comment.