From 1dc67bd4398b78b79d4a16b764465418d98ab8a0 Mon Sep 17 00:00:00 2001 From: anatol-sialitski <53557255+anatol-sialitski@users.noreply.github.com> Date: Thu, 5 Sep 2024 10:24:29 +0200 Subject: [PATCH] API descriptor can be optional #10673 (#10674) API descriptor can be optional #10673 --- .../portal/impl/handler/SlashApiHandler.java | 109 +++++++++--------- .../impl/handler/SlashApiHandlerTest.java | 40 +------ 2 files changed, 54 insertions(+), 95 deletions(-) diff --git a/modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/handler/SlashApiHandler.java b/modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/handler/SlashApiHandler.java index 7510544ed34..2260dd78737 100644 --- a/modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/handler/SlashApiHandler.java +++ b/modules/portal/portal-impl/src/main/java/com/enonic/xp/portal/impl/handler/SlashApiHandler.java @@ -123,7 +123,7 @@ public SlashApiHandler( @Reference final ControllerScriptFactory controllerScrip @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC) public void addApiHandler( final UniversalApiHandler apiHandler, final Map properties ) { - final ApiDescriptor apiDescriptor = resolveDynamicApiDescriptor( properties ); + final ApiDescriptor apiDescriptor = createDynamicApiDescriptor( properties ); this.dynamicApiHandlers.put( apiDescriptor.getKey(), new UniversalApiHandlerWrapper( apiHandler, apiDescriptor ) ); } @@ -173,18 +173,34 @@ public WebResponse handle( final WebRequest webRequest ) if ( dynamicApiHandler != null ) { - return execute( portalRequest, dynamicApiHandler.apiDescriptor, restPath, + return execute( portalRequest, dynamicApiHandler.apiDescriptor.getKey(), restPath, () -> dynamicApiHandler.apiHandler.handle( portalRequest ) ); } + else + { + final DescriptorKey descriptorKey = resolveDescriptorKey( applicationKey, apiKey, portalRequest ); + return execute( portalRequest, descriptorKey, restPath, () -> executeController( portalRequest, descriptorKey ) ); + } + } - final ApiDescriptor apiDescriptor = resolveApiDescriptor( applicationKey, apiKey ); - - if ( !verifyRequestMounted( apiDescriptor, portalRequest ) ) + private DescriptorKey resolveDescriptorKey( final ApplicationKey applicationKey, final String apiKey, + final PortalRequest portalRequest ) + { + // If the unnamed API is present, then the named APIs are skipped. + DescriptorKey descriptorKey = DescriptorKey.from( applicationKey, UNNAMED_API_KEY ); + ApiDescriptor apiDescriptor = apiDescriptorService.getByKey( descriptorKey ); + if ( apiDescriptor == null ) + { + descriptorKey = DescriptorKey.from( applicationKey, apiKey ); + apiDescriptor = apiDescriptorService.getByKey( descriptorKey ); + } + if ( apiDescriptor != null ) { - throw WebException.notFound( String.format( "API [%s] is not mounted", apiDescriptor.getKey() ) ); + verifyAccessToApi( apiDescriptor ); } + verifyRequestMounted( descriptorKey, portalRequest ); - return execute( portalRequest, apiDescriptor, restPath, () -> executeController( portalRequest, apiDescriptor ) ); + return descriptorKey; } private UniversalApiHandlerWrapper resolveDynamicApiHandler( final ApplicationKey applicationKey, final String apiKey, @@ -203,16 +219,12 @@ private UniversalApiHandlerWrapper resolveDynamicApiHandler( final ApplicationKe } verifyAccessToApi( handlerWrapper.apiDescriptor ); - - if ( !verifyRequestMounted( handlerWrapper.apiDescriptor, portalRequest ) ) - { - throw WebException.notFound( String.format( "API [%s] is not mounted", handlerWrapper.apiDescriptor.getKey() ) ); - } + verifyRequestMounted( handlerWrapper.apiDescriptor.getKey(), portalRequest ); return handlerWrapper; } - private WebResponse execute( final PortalRequest portalRequest, final ApiDescriptor apiDescriptor, final String restPath, + private WebResponse execute( final PortalRequest portalRequest, final DescriptorKey descriptorKey, final String restPath, final Supplier supplier ) throws Exception { @@ -223,38 +235,41 @@ private WebResponse execute( final PortalRequest portalRequest, final ApiDescrip } return Tracer.traceEx( trace, () -> { final WebResponse response = handleAPIRequest( portalRequest, supplier ); - addTranceInfo( trace, apiDescriptor.getKey(), restPath, response ); + addTranceInfo( trace, descriptorKey, restPath, response ); return response; } ); } - private boolean verifyRequestMounted( final ApiDescriptor apiDescriptor, final PortalRequest portalRequest ) + private void verifyRequestMounted( final DescriptorKey descriptorKey, final PortalRequest portalRequest ) { final String rawPath = portalRequest.getRawPath(); + boolean isMounted = false; + if ( portalRequest.getEndpointPath() == null && rawPath.startsWith( "/api/" ) ) { - return true; + isMounted = true; } else if ( portalRequest.getEndpointPath() != null && rawPath.startsWith( "/site/" ) || rawPath.startsWith( "/admin/site/" ) ) { - return verifyRequestMountedOnSites( apiDescriptor, portalRequest ); + isMounted = verifyRequestMountedOnSites( descriptorKey, portalRequest ); } else if ( portalRequest.getEndpointPath() != null && rawPath.startsWith( "/webapp/" ) ) { - return verifyPathMountedOnWebapps( apiDescriptor, portalRequest ); + isMounted = verifyPathMountedOnWebapps( descriptorKey, portalRequest ); } else if ( portalRequest.getEndpointPath() != null && rawPath.startsWith( "/admin/" ) ) { - return verifyPathMountedOnAdminTool( apiDescriptor, portalRequest ); + isMounted = verifyPathMountedOnAdminTool( descriptorKey, portalRequest ); } - else + + if ( !isMounted ) { - return false; + throw WebException.notFound( String.format( "API [%s] is not mounted", descriptorKey ) ); } } - private boolean verifyPathMountedOnAdminTool( final ApiDescriptor apiDescriptor, final PortalRequest portalRequest ) + private boolean verifyPathMountedOnAdminTool( final DescriptorKey descriptorKey, final PortalRequest portalRequest ) { final Matcher matcher = MOUNT_ADMINTOOL_API_PATTERN.matcher( portalRequest.getRawPath() ); if ( !matcher.find() ) @@ -265,8 +280,7 @@ private boolean verifyPathMountedOnAdminTool( final ApiDescriptor apiDescriptor, final ApplicationKey applicationKey = HandlerHelper.resolveApplicationKey( matcher.group( "appKey" ) ); final String tool = matcher.group( "tool" ); - final DescriptorKey descriptorKey = DescriptorKey.from( applicationKey, tool ); - final AdminToolDescriptor adminToolDescriptor = adminToolDescriptorService.getByKey( descriptorKey ); + final AdminToolDescriptor adminToolDescriptor = adminToolDescriptorService.getByKey( DescriptorKey.from( applicationKey, tool ) ); if ( adminToolDescriptor == null ) { return false; @@ -274,11 +288,11 @@ private boolean verifyPathMountedOnAdminTool( final ApiDescriptor apiDescriptor, return adminToolDescriptor.getApiMounts() .stream() - .anyMatch( descriptor -> descriptor.getApiKey().equals( apiDescriptor.getKey().getName() ) && - descriptor.getApplicationKey().equals( apiDescriptor.getKey().getApplicationKey() ) ); + .anyMatch( descriptor -> descriptor.getApiKey().equals( descriptorKey.getName() ) && + descriptor.getApplicationKey().equals( descriptorKey.getApplicationKey() ) ); } - private boolean verifyPathMountedOnWebapps( final ApiDescriptor apiDescriptor, final PortalRequest portalRequest ) + private boolean verifyPathMountedOnWebapps( final DescriptorKey descriptorKey, final PortalRequest portalRequest ) { final Matcher webappMatcher = MOUNT_WEBAPP_ENDPOINT_PATTERN.matcher( portalRequest.getRawPath() ); if ( !webappMatcher.find() ) @@ -296,11 +310,11 @@ private boolean verifyPathMountedOnWebapps( final ApiDescriptor apiDescriptor, f return webappDescriptor.getApiMounts() .stream() - .anyMatch( descriptor -> descriptor.getApiKey().equals( apiDescriptor.getKey().getName() ) && - descriptor.getApplicationKey().equals( apiDescriptor.getKey().getApplicationKey() ) ); + .anyMatch( descriptor -> descriptor.getApiKey().equals( descriptorKey.getName() ) && + descriptor.getApplicationKey().equals( descriptorKey.getApplicationKey() ) ); } - private boolean verifyRequestMountedOnSites( final ApiDescriptor apiDescriptor, final PortalRequest portalRequest ) + private boolean verifyRequestMountedOnSites( final DescriptorKey descriptorKey, final PortalRequest portalRequest ) { final ContentResolverResult contentResolverResult = new ContentResolver( contentService ).resolve( portalRequest ); @@ -321,8 +335,8 @@ private boolean verifyRequestMountedOnSites( final ApiDescriptor apiDescriptor, { final ApiMountDescriptor apiMountDescriptor = siteDescriptor.getApiDescriptors() .stream() - .filter( descriptor -> descriptor.getApiKey().equals( apiDescriptor.getKey().getName() ) && - descriptor.getApplicationKey().equals( apiDescriptor.getKey().getApplicationKey() ) ) + .filter( descriptor -> descriptor.getApiKey().equals( descriptorKey.getName() ) && + descriptor.getApplicationKey().equals( descriptorKey.getApplicationKey() ) ) .findAny() .orElse( null ); @@ -378,23 +392,7 @@ private WebResponse handleAPIRequest( final PortalRequest portalRequest, final S } } - private ApiDescriptor resolveApiDescriptor( final ApplicationKey applicationKey, final String apiKey ) - { - // If the unnamed API is present, then the named APIs are skipped. - ApiDescriptor apiDescriptor = apiDescriptorService.getByKey( DescriptorKey.from( applicationKey, UNNAMED_API_KEY ) ); - if ( apiDescriptor == null ) - { - final DescriptorKey descriptorKey = DescriptorKey.from( applicationKey, apiKey ); - apiDescriptor = apiDescriptorService.getByKey( descriptorKey ); - if ( apiDescriptor == null ) - { - throw WebException.notFound( String.format( "API [%s] not found", descriptorKey ) ); - } - } - return verifyAccessToApi( apiDescriptor ); - } - - private ApiDescriptor verifyAccessToApi( final ApiDescriptor apiDescriptor ) + private void verifyAccessToApi( final ApiDescriptor apiDescriptor ) { final PrincipalKeys principals = ContextAccessor.current().getAuthInfo().getPrincipals(); if ( !apiDescriptor.isAccessAllowed( principals ) ) @@ -403,7 +401,6 @@ private ApiDescriptor verifyAccessToApi( final ApiDescriptor apiDescriptor ) String.format( "You don't have permission to access \"%s\" API for \"%s\"", apiDescriptor.getKey().getName(), apiDescriptor.getKey().getApplicationKey() ) ); } - return apiDescriptor; } private PortalRequest createPortalRequest( final WebRequest webRequest, final ApplicationKey applicationKey ) @@ -417,9 +414,9 @@ private PortalRequest createPortalRequest( final WebRequest webRequest, final Ap return portalRequest; } - private PortalResponse executeController( final PortalRequest req, final ApiDescriptor apiDescriptor ) + private PortalResponse executeController( final PortalRequest req, final DescriptorKey descriptorKey ) { - final ControllerScript script = getScript( apiDescriptor ); + final ControllerScript script = getScript( descriptorKey ); final PortalResponse res = script.execute( req ); final WebSocketConfig webSocketConfig = res.getWebSocket(); @@ -427,7 +424,7 @@ private PortalResponse executeController( final PortalRequest req, final ApiDesc if ( webSocketContext != null && webSocketConfig != null ) { final WebSocketEndpoint webSocketEndpoint = - newWebSocketEndpoint( webSocketConfig, script, apiDescriptor.getKey().getApplicationKey() ); + newWebSocketEndpoint( webSocketConfig, script, descriptorKey.getApplicationKey() ); try { webSocketContext.apply( webSocketEndpoint ); @@ -452,9 +449,9 @@ private WebSocketEndpoint newWebSocketEndpoint( final WebSocketConfig config, fi return new WebSocketEndpointImpl( config, () -> script ); } - private ControllerScript getScript( final ApiDescriptor apiDescriptor ) + private ControllerScript getScript( final DescriptorKey descriptorKey ) { - final ResourceKey script = ApiDescriptor.toResourceKey( apiDescriptor.getKey(), "js" ); + final ResourceKey script = ApiDescriptor.toResourceKey( descriptorKey, "js" ); final Trace trace = Tracer.current(); if ( trace != null ) { @@ -471,7 +468,7 @@ private WebResponse handleError( final WebRequest webRequest, final Exception e return webResponse; } - private ApiDescriptor resolveDynamicApiDescriptor( final Map properties ) + private ApiDescriptor createDynamicApiDescriptor( final Map properties ) { final ApplicationKey applicationKey = ApplicationKey.from( (String) properties.get( "applicationKey" ) ); final String apiKey = Objects.requireNonNullElse( (String) properties.get( "apiKey" ), UNNAMED_API_KEY ); diff --git a/modules/portal/portal-impl/src/test/java/com/enonic/xp/portal/impl/handler/SlashApiHandlerTest.java b/modules/portal/portal-impl/src/test/java/com/enonic/xp/portal/impl/handler/SlashApiHandlerTest.java index 75773d3aff7..66003707fd5 100644 --- a/modules/portal/portal-impl/src/test/java/com/enonic/xp/portal/impl/handler/SlashApiHandlerTest.java +++ b/modules/portal/portal-impl/src/test/java/com/enonic/xp/portal/impl/handler/SlashApiHandlerTest.java @@ -199,21 +199,6 @@ void testHttpOptions() assertEquals( "GET,POST,HEAD,OPTIONS,PUT,DELETE,TRACE", res.getHeaders().get( "Allow" ) ); } - @Test - void testHandleApiNotFound() - { - final WebRequest webRequest = mock( WebRequest.class ); - when( webRequest.getMethod() ).thenReturn( HttpMethod.GET ); - when( webRequest.getEndpointPath() ).thenReturn( null ); - when( webRequest.getRawPath() ).thenReturn( "/api/com.enonic.app.myapp/api-key" ); - - when( apiDescriptorService.getByKey( any( DescriptorKey.class ) ) ).thenReturn( null ); - - WebException ex = assertThrows( WebException.class, () -> this.handler.handle( webRequest ) ); - assertEquals( HttpStatus.NOT_FOUND, ex.getStatus() ); - assertEquals( "API [com.enonic.app.myapp:api-key] not found", ex.getMessage() ); - } - @Test void testHandleApiAccessDenied() { @@ -501,8 +486,7 @@ void testWebappMountOwnUnnamedAPI() { final ApplicationKey webappApplicationKey = ApplicationKey.from( "com.enonic.app.mywebapp" ); - final ApiMountDescriptor unnamedApiInWebapp = - ApiMountDescriptor.create().applicationKey( webappApplicationKey ).build(); + final ApiMountDescriptor unnamedApiInWebapp = ApiMountDescriptor.create().applicationKey( webappApplicationKey ).build(); final WebappDescriptor webappDescriptor = WebappDescriptor.create() .applicationKey( webappApplicationKey ) @@ -750,28 +734,6 @@ void testAddAndRemoveDynamicApiHandler() handler.removeApiHandler( myUniversalApiHandler ); } - @Test - void testHandleDynamicApiHandlerDoesNotRegister() - { - final ApplicationKey apiApplicationKey = ApplicationKey.from( "com.enonic.app.external.app" ); - final ApplicationKey applicationKey = ApplicationKey.from( "com.enonic.app.myapp" ); - final DescriptorKey descriptorKey = DescriptorKey.from( applicationKey, "mytool" ); - final AdminToolDescriptor toolDescriptor = AdminToolDescriptor.create() - .displayName( "My Tool" ) - .key( descriptorKey ) - .apiMounts( - ApiMountDescriptors.from( ApiMountDescriptor.create().applicationKey( apiApplicationKey ).apiKey( "myapi" ).build() ) ) - .build(); - - when( adminToolDescriptorService.getByKey( eq( descriptorKey ) ) ).thenReturn( toolDescriptor ); - - request.setEndpointPath( "/_/com.enonic.app.external.app/myapi" ); - request.setRawPath( "/admin/com.enonic.app.myapp/mytool/_/com.enonic.app.external.app/myapi" ); - - WebException ex = assertThrows( WebException.class, () -> this.handler.handle( request ) ); - assertEquals( HttpStatus.NOT_FOUND, ex.getStatus() ); - } - @Test void testHandleDynamicApiHandlerWhichDoesNotMountToAPI() {