[MGNLREST-138] Cannot decorate the delivery endpoint Created: 30/Oct/17  Updated: 04/Dec/17  Resolved: 14/Nov/17

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

Type: Bug Priority: Critical
Reporter: Christopher Zimmermann Assignee: Mikaël Geljić
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: 0d
Time Spent: 1d 7h
Original Estimate: 4.5h

Attachments: Zip Archive light-modules-decoration.zip    
Issue Links:
Relates
relates to MAGNOLIA-7193 Number keys are not supported in defi... Closed
relates to MAGNOLIA-7194 After decoration, new provider with i... Closed
relates to MGNLREST-157 Endpoint doesn't get updated after de... 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
Date of First Response:
Epic Link: REST Queries
Sprint: Saigon 120, Saigon 121, Saigon 122
Story Points: 5

 Description   

Attempts to decorate the delivery endpoint fail.
Decoration is very important since only one instance of the delivery endpoint can be created. Decoration is the mechanism by which each module can end the endpoint configurations that they require.

Attempted configuration:

Original config in module "rest-test":
/light-modules/rest-test/restEndpoints/rest1.yaml

class: info.magnolia.rest.delivery.jcr.v1.JcrDeliveryEndpointDefinition
params:
  contacts:
    workspace: contacts
    nodeTypes:
      0: mgnl:contact

  # tours:
  #   workspace: tours

Attempt at decoration:
/light-modules/add-endpoints/decorations/rest-test/restEndpoints/rest1.yaml

params:
  tours:
    workspace: tours

(See attachment for full configuration)

Results in

  • In the decorations app - the "delivery" endpoint is visible, but cannot be expanded.
  • In the problems view "Illegal state: Only support maps with String keys 0 in {0=mgnl:contact}
  • Requests to /.rest/delivery/tours/v1/ result in response No workspace-params entry for endpoint prefix 'tours' and an exception in the log.
  • Requests to /.rest/delivery/contacts/v1/ work fine.

Note:

If I wrap the nodetypes in the original with quotes, then the definitions app displays the endpoint properly and I can expand it, there are no problems in the problems view, but the actual requests still fail with exceptions.



 Comments   
Comment by Christoph Meier [ 31/Oct/17 ]

My findings are the followings:

#1 You can decorate the endpoint, but you have to restart the server to make sure the endpoint is aware of the changes.

#2 The endpoint-definition as known in the definitions-app gets properly updated when decorating - without a restart.
But the member variable params on the class JcrDeliveryEndpoint doesn't get updated, if the change orignates from a decoration.
(But params gets updated, when changing the endpoint-definition.yaml without restart.).

#3 The decorating module (example-name: adapt-rest) should define a module dependency to the module with the endpoint-definition.yaml (example-module-name: define-rest) otherwise decoration may be applied or not (during start-up).
Further more it looks like the module define-rest also requires a module descriptor to define its version to ensure the dependency management works fine.

Obervation #3 is not really relevant in the context of the ticket.

Conclusion: The endpoint must read the endpoint-definition from the registry to make sure it always gets the latest state.
Properly implementing this would already contain (half of) the implementation for multiple deliveryEndpoint definitions.

Comment by Christoph Meier [ 31/Oct/17 ]

I have a "work-around" on a branch on my fork

Since I inject EndpointDefinitionRegistry instead of extending AbstractEndpoint, constructor has changed - which also leads to changes on JcrDeliveryEndpointTest - which I have not done so far and which is fully commented atm. :-|

Somebody else must decide whether my workaround could be a tmp.-solution for 2.0.
If yes, the test class must be adapted too.

I am unsure whether the issue can be fixed - without reading from the registry.
If we use the registry - the path to multiple delivery endpoint definitions becomes quite short.
Whatever ... mgeljic or somebody else with a deeper understanding should check this.
Amen

Comment by Christopher Zimmermann [ 01/Nov/17 ]

I can confirm that after a restart my decoration is picked up!
I can also confirm that changes made to the decoration during runtime are

  • Not picked up when returning a result from the endpoint. (my response stays the same when I change the "limit" parameter.
  • But the changes are visible in the definitions app.

(So the actual endpoint behaviour is out of sync with what is reflected in the definitions app.)

Comment by Mikaël Geljić [ 09/Nov/17 ]

Illegal state: Only support maps with String keys 0 in

Unknown macro: {0=mgnl}

Unrelated issue in decoration—regular YamlReader doesn't seem to have any issue with numeral keys. Meanwhile quote your key or use the list syntax as the key is irrelevant anyway.

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