Conversation
| @@ -1,4 +1,4 @@ | |||
| package org.orcid.api.publicV3.server.security.impl; | |||
There was a problem hiding this comment.
Hey @cryptalith why did you moved this class from the pub api package? you moved it to the orcid-core lib so any other env will now have access to it, which is not ideal
There was a problem hiding this comment.
Moved the interface and implementation back under the public v3 security package, but placed them in orcid-api-common so both registry web and public API can share the same visibility-filtering logic without orcid-web depending on the orcid-pub-web WAR
| @@ -1,4 +1,4 @@ | |||
| package org.orcid.api.publicV3.server.security; | |||
There was a problem hiding this comment.
Hey @cryptalith why did you moved this class from the pub api package? you moved it to the orcid-core lib so any other env will now have access to it, which is not ideal
There was a problem hiding this comment.
Answered in the previous thread.
|
|
||
| <bean id="orcidCoreExceptionMapper" class="org.orcid.core.exception.OrcidCoreExceptionMapper" /> | ||
|
|
||
| <bean id="publicAPISecurityManagerV3" class="org.orcid.core.api.publicapi.v3.security.impl.PublicAPISecurityManagerV3Impl" /> |
There was a problem hiding this comment.
I think you dont need this bean here, where are you using it and why do you need it?
There was a problem hiding this comment.
As mentioned in the previous thread, publicAPISecurityManagerV3 is now removed from core, so I am also removing this.
| private PublicRecordUtils() { | ||
| } | ||
|
|
||
| public static Record getPublicRecord(String orcid, RecordManagerReadOnly recordManagerReadOnly, OrcidSecurityManager orcidSecurityManager, |
There was a problem hiding this comment.
Hey @cryptalith , getPublicRecord should have only 2 params, the orcid and the boolean filterVersionOfIdentifiers, then, all the beans that are used to generate and validate the data should be @resources of this class.
Annotate the class definition with @Component and that will be enought to make the @Resource work
| import org.springframework.web.bind.annotation.RestControllerAdvice; | ||
|
|
||
| @RestControllerAdvice(assignableTypes = PublicRecordApiController.class) | ||
| public class PublicRecordApiExceptionHandler { |
There was a problem hiding this comment.
@cryptalith did you tested that this code runs? Im not sure if this code is necessary or not.
There was a problem hiding this comment.
Yes, this can be tested by navigating to a deprecated orcid id print page.
As the PublicRecordApiController is a Spring MVC @controller. Requests to /{orcid}/record are handled by Spring’s not by the JAX-RS stack where OrcidExceptionMapper run for the public API.
So for this endpoint the @RestControllerAdvice is the Spring-side equivalent: it turns exceptions from that controller only into HTTP responses.
cryptalith
force-pushed
the
lmendoza/PD-5278
branch
from
b0703e0 to
d3c8ab5
Compare
2 participants
No description provided.