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

Feature/#777 #783

Merged
merged 8 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -24,7 +24,7 @@ import au.org.ala.plugins.openapi.Path
type = SecuritySchemeType.HTTP,
scheme = "bearer"
)
@RequireApiKey()

class ApiController extends BaseController {
static namespace = "v1"
static allowedSortFields = ['scientificNameLower', 'lastUpdated', 'dateCreated']
Expand All @@ -34,6 +34,7 @@ class ApiController extends BaseController {
MapService mapService
ApiService apiService

@RequireApiKey()
Copy link
Contributor

Choose a reason for hiding this comment

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

why is requireapikey required? Similarly other below actions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

previous ApiController class had @RequireApiKey. so I put it into all method except api/opus/list

Copy link
Contributor

Choose a reason for hiding this comment

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

Keep security standard for all actions.

@Path("/api/opus/{opusId}")
@Operation(
summary = "Get collection (opus) details",
Expand Down Expand Up @@ -106,6 +107,41 @@ class ApiController extends BaseController {
}
}

@Path("/api/opus/list")
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to /api/opus for consistency with other APIs

@Operation(
summary = "Get all collection (opus) details",
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps better to reword it to Get all public collections

operationId = "/api/opus/list",
method = "GET",
responses = [
@ApiResponse(
responseCode = "200",
content = @Content(
mediaType = "application/json",
array = @ArraySchema(
schema = @Schema(
implementation = OpusResponse.class
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with CollectionList since OpusResponse has a lost more properties than what is returned by this API.

)
)
)
),
@ApiResponse(responseCode = "400",
description = "opusId is a required parameter"),
@ApiResponse(responseCode = "403",
description = "You do not have the necessary permissions to perform this action."),
@ApiResponse(responseCode = "405",
description = "An unexpected error has occurred while processing your request."),
@ApiResponse(responseCode = "404",
description = "Collection not found"),
@ApiResponse(responseCode = "500",
description = "An unexpected error has occurred while processing your request.")
]
)
def getListCollections () {
List opus = profileService.getOpusList() as List
Copy link
Contributor

Choose a reason for hiding this comment

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

Reuse profileService.getOpus(). No need to implement a custom code in profile-hub or profile-service.

render opus as JSON
}

@RequireApiKey()
@Path("/api/opus/{opusId}/profile")
@Operation(
summary = "List profiles in a collection",
Expand Down Expand Up @@ -245,6 +281,7 @@ class ApiController extends BaseController {
}
}

@RequireApiKey()
@Path("/api/opus/{opusId}/profile/{profileId}")
@Operation(
summary = "Get a profile in a collection",
Expand Down Expand Up @@ -333,6 +370,7 @@ class ApiController extends BaseController {
}
}

@RequireApiKey()
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove. applies to other implementations.

@Path("/api/opus/{opusId}/profile/{profileId}/draft")
@Operation(
summary = "Get a draft profile in a collection",
Expand Down Expand Up @@ -416,6 +454,7 @@ class ApiController extends BaseController {
}
}

@RequireApiKey()
@Path("/api/opus/{opusId}/profile/{profileId}/image")
@Operation(
summary = "Get images associated with a profile",
Expand Down Expand Up @@ -510,6 +549,7 @@ class ApiController extends BaseController {
}
}

@RequireApiKey()
@Path("/api/opus/{opusId}/profile/{profileId}/attribute/{attributeId}")
@Operation(
summary = "Get attributes of a profile in a collection",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ class ApiInterceptor {
Class controllerClass = controller?.clazz
def method = controllerClass?.getMethod(actionName, [] as Class[])

if (method.name == "getListCollections") return true
Copy link
Contributor

Choose a reason for hiding this comment

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

should not escape api interceptor logic.


if (authorization) {
if (params.opusId && (opus = profileService.getOpus(params.opusId))) {
params.isOpusPrivate = opus.privateCollection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ class UrlMappings {
get "/opus/$opusId/profile/$profileId/image" (version: "1.0", controller: "api", action: "getImages", namespace: "v1")
get "/opus/$opusId/profile/$profileId/attribute/$attributeId" (version: "1.0", controller: "api", action: "getAttributes", namespace: "v1")
get "/opus/$opusId/profile/$profileId/draft" (version: "1.0", controller: "api", action: "getDraftProfile", namespace: "v1")
get "/opus/list" (version: "1.0", controller: "api", action: "getListCollections", namespace: "v1")
Copy link
Contributor

Choose a reason for hiding this comment

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

should be /api/opus

}

"/openapi/$action?/$id?(.$format)?"(controller: "openApi")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ class ProfileService {
webServiceWrapperService.get("${grailsApplication.config.profile.service.url}/opus/${encPath(opusId)}", [:], ContentType.APPLICATION_JSON, true, false, getCustomHeaderWithUserId())?.resp
}

def getOpusList() {
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above.

webServiceWrapperService.get("${grailsApplication.config.profile.service.url}/opus/list", [:], ContentType.APPLICATION_JSON, false, false, null)?.resp
}


def updateOpus(String opusId, Map json) {
webService.post("${grailsApplication.config.profile.service.url}/opus/${encPath(opusId)}", json, [:], ContentType.APPLICATION_JSON, true, false, getCustomHeaderWithUserId())
}
Expand Down
11 changes: 11 additions & 0 deletions src/test/groovy/au/org/ala/profile/api/ApiControllerSpec.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,15 @@ class ApiControllerSpec extends Specification implements ControllerUnitTest<ApiC
'opus1' | '123' | null | 404
'opus1' | '123' | 'a,b' | 200
}

void "getOpusList should be provided"() {
setup:
profileService.getOpusList()>> [[uuid: 'abc',shortName:'alatest',title:'title1',desciption:'desc1',thubnailUrl:'test.png']]

when:
controller.getListCollections()

then:
response.status == 200
}
}
Loading