[MGNLREST-156] SwaggerRestDispatcherServlet (re)register endpoint without provider validation. Created: 01/Dec/17  Updated: 25/Dec/17  Resolved: 19/Dec/17

Status: Closed
Project: Magnolia REST Framework
Component/s: tools
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Neutral
Reporter: Dai Ha Assignee: Dai Ha
Resolution: Won't Fix Votes: 0
Labels: None
Remaining Estimate: 0d
Time Spent: 2d
Original Estimate: 2d

Attachments: Zip Archive endpoints.zip     Zip Archive rest-tool_w_deps.zip    
Issue Links:
Relates
relates to MAGNOLIA-7194 After decoration, new provider with i... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Sprint: Saigon 124, Saigon 125, Saigon 126, Saigon 127
Story Points: 3

 Description   

We have issue with SwaggerRestDispatcherServlet which executes delegation calls to info.magnolia.rest.RestDispatcherServlet methods. Unfortunately, those call is not secured with provider.isValid check --> endpoint ends up failed to re-register.

Prefer:
info.magnolia.rest.tools.SwaggerRestDispatcherServlet#onEndpointRegistered
info.magnolia.rest.tools.SwaggerRestDispatcherServlet#onEndpointReregistered

Stack trace:

2017-12-13 10:41:49,649 WARN  fig.source.yaml.decoration.YamlDefinitionDecorator: 
Severity type:   MAJOR 
Title:           /: [Definition decoration] - Failed to parse decoration data from [/1_decor/decorations/rest-content-delivery/restEndpoints/restDelivery.yaml] 
Problem details: Illegal state: Only support maps with String keys 0 in {0=mgnl:page, 1=mgnl:area, 2=mgnl:component}
2017-12-13 10:41:49,653 ERROR info.magnolia.event.SimpleEventBus                : Exception caught when dispatching info.magnolia.rest.registry.EndpointDefinitionRegistryEvent with info.magnolia.rest.tools.SwaggerRestDispatcherServlet eventHandler.
info.magnolia.objectfactory.MgnlInstantiationException: Unable to resolve parameters for constructor public info.magnolia.rest.delivery.jcr.v1.JcrDeliveryEndpoint(info.magnolia.rest.delivery.jcr.v1.JcrDeliveryEndpointDefinition,javax.inject.Provider). Unresolved parameter(s) are: 1st parameter which is of type info.magnolia.rest.delivery.jcr.v1.JcrDeliveryEndpointDefinition
	at info.magnolia.objectfactory.ObjectManufacturer.newInstance(ObjectManufacturer.java:97) ~[magnolia-core-5.6.jar:?]
	at info.magnolia.objectfactory.guice.GuiceComponentProvider.newInstanceWithParameterResolvers(GuiceComponentProvider.java:132) ~[magnolia-core-5.6.jar:?]
	at info.magnolia.objectfactory.guice.GuiceComponentProvider.newInstanceWithParameterResolvers(GuiceComponentProvider.java:119) ~[magnolia-core-5.6.jar:?]
	at info.magnolia.objectfactory.guice.GuiceComponentProvider.newInstance(GuiceComponentProvider.java:114) ~[magnolia-core-5.6.jar:?]
	at info.magnolia.rest.RestDispatcherServlet.instantiateEndpoint(RestDispatcherServlet.java:211) ~[magnolia-rest-integration-2.0.jar:?]
	at info.magnolia.rest.RestDispatcherServlet.registerEndpoint(RestDispatcherServlet.java:189) ~[magnolia-rest-integration-2.0.jar:?]
	at info.magnolia.rest.RestDispatcherServlet.onEndpointReregistered(RestDispatcherServlet.java:155) ~[magnolia-rest-integration-2.0.jar:?]
	at info.magnolia.rest.tools.SwaggerRestDispatcherServlet.onEndpointReregistered(SwaggerRestDispatcherServlet.java:161) ~[magnolia-rest-tools-2.0.jar:?]
	at info.magnolia.rest.registry.EndpointDefinitionRegistryEvent.dispatch(EndpointDefinitionRegistryEvent.java:106) ~[magnolia-rest-integration-2.0.jar:?]
	at info.magnolia.rest.registry.EndpointDefinitionRegistryEvent.dispatch(EndpointDefinitionRegistryEvent.java:55) ~[magnolia-rest-integration-2.0.jar:?]
	at info.magnolia.event.SimpleEventBus.fireEvent(SimpleEventBus.java:75) ~[magnolia-core-5.6.jar:?]
	at info.magnolia.rest.registry.EndpointDefinitionRegistry.lambda$addDecorator$8(EndpointDefinitionRegistry.java:137) ~[magnolia-rest-integration-2.0.jar:?]
	at java.util.Optional.ifPresent(Optional.java:159) ~[?:1.8.0_131]
	at info.magnolia.rest.registry.EndpointDefinitionRegistry.addDecorator(EndpointDefinitionRegistry.java:137) ~[magnolia-rest-integration-2.0.jar:?]
	at info.magnolia.config.source.yaml.AbstractFileResourceConfigurationSource$DefinitionDecoratorResolutionFunction.doWith(AbstractFileResourceConfigurationSource.java:302) ~[magnolia-configuration-5.6.jar:?]
	at info.magnolia.config.source.yaml.AbstractFileResourceConfigurationSource$DefinitionDecoratorResolutionFunction.doWith(AbstractFileResourceConfigurationSource.java:290) ~[magnolia-configuration-5.6.jar:?]
	at info.magnolia.resourceloader.util.VoidFunction.apply(VoidFunction.java:49) ~[magnolia-resource-loader-5.6.jar:?]
	at info.magnolia.config.source.yaml.AbstractFileResourceConfigurationSource.lambda$startDecoration$3(AbstractFileResourceConfigurationSource.java:190) ~[magnolia-configuration-5.6.jar:?]
	at info.magnolia.resourceloader.layered.LayeredResourceOrigin$RelayingChangeHandler.onResourceChanged(LayeredResourceOrigin.java:285) [magnolia-resource-loader-5.6.jar:?]
	at info.magnolia.resourceloader.AbstractResourceOrigin.dispatchResourceChange(AbstractResourceOrigin.java:132) [magnolia-resource-loader-5.6.jar:?]
	at info.magnolia.resourceloader.file.FileWatcherCallback.dispatchResourceChange(FileWatcherCallback.java:117) [magnolia-resource-loader-5.6.jar:?]
	at info.magnolia.resourceloader.file.FileWatcherCallback.modified(FileWatcherCallback.java:88) [magnolia-resource-loader-5.6.jar:?]
	at info.magnolia.dirwatch.DirectoryWatcher.processEvent(DirectoryWatcher.java:307) [magnolia-resource-loader-5.6.jar:?]
	at info.magnolia.dirwatch.DirectoryWatcher.run(DirectoryWatcher.java:253) [magnolia-resource-loader-5.6.jar:?]
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) [?:1.8.0_131]
	at java.util.concurrent.FutureTask.run(FutureTask.java:266) [?:1.8.0_131]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) [?:1.8.0_131]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) [?:1.8.0_131]
	at java.lang.Thread.run(Thread.java:748) [?:1.8.0_131]

Environments to re-produce:
1. Magnolia 5.6 release:
+ bundle loaded with rest-tool 2.0
2. Magnolia 5.6.1:
+ bundle loaded with rest-tool 2.0 (before the fix MAGNOLIA-7193) (this ticket include yaml definition and yaml decoration that can re-produce issue easily before MAGNOLIA-7193. After MAGNOLIA-7193, issue still possible to happens but would harder to make it occurs.)

Step to reproduce:
1) Get bundle start successfully with rest-tool enabled (tried with release bundles, can't add rest-tool without manually provide swagger dependencies).
2) Add yaml definition module for rest
3) Add yaml decoration for previously add rest definition --> this definition would not effect due to MGNLREST-157
4) Try to modify decoration file --> without rest-tool, this modification would result in an effective decoration. With rest-tool, this is causing above exception and original endpoint unregister.



 Comments   
Comment by Dai Ha [ 14/Dec/17 ]

Comparison with/without rest-tool module:

With rest-tool:
+ on new rest definition added -> info.magnolia.rest.RestDispatcherServlet#onEndpointRegistered
+ on rest definition edited -> info.magnolia.rest.RestDispatcherServlet#onEndpointRegistered

+ on decoration added -> info.magnolia.rest.RestDispatcherServlet#onEndpointReregistered
+ on decoration edited -> info.magnolia.rest.RestDispatcherServlet#onEndpointReregistered

Withou rest-tool:
+ on new rest definition added -> info.magnolia.rest.RestDispatcherServlet#onEndpointRegistered
+ on rest definition edited -> info.magnolia.rest.RestDispatcherServlet#onEndpointRegistered

+ on decoration added -> info.magnolia.rest.RestDispatcherServlet#onEndpointReregistered
+ on decoration edited -> info.magnolia.rest.RestDispatcherServlet#onEndpointReregistered

So:
+ for add/edit rest definition, endpoint can be removed if definition provider is invalid.

Comment by Dai Ha [ 19/Dec/17 ]

Close this ticket as Won't fix as:
+ It's acceptable that endpoint could unregister due to failed decoration -> User then can be aware of the failure and has a proper fix.
+ Align the behavior in case of server restart -> with invalid provider, server will start without endpoint registered.

Generated at Mon Feb 12 06:57:10 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.