[MGNLREST-574] LegacyPageWriter in pages returns empty areas if the endpoint definition not including system properties Created: 15/Nov/22 Updated: 16/May/23 Resolved: 12/Dec/22 |
|
| Status: | Closed |
| Project: | Magnolia REST Framework |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 3.0.0 |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Canh Nguyen | Assignee: | Oanh Thai Hoang |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | None | ||
| Σ Remaining Estimate: | Not Specified | Remaining Estimate: | Not Specified |
| Σ Time Spent: | 1d 1.5h | Time Spent: | 1d 1.5h |
| Σ Original Estimate: | Not Specified | Original Estimate: | Not Specified |
| Attachments: |
|
|||||||||||||||||||||||||
| Issue Links: |
|
|||||||||||||||||||||||||
| Sub-Tasks: |
|
|||||||||||||||||||||||||
| 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: | Norsu delivery endpoint (phase 1) | |||||||||||||||||||||||||
| Team: | ||||||||||||||||||||||||||
| Work Started: | ||||||||||||||||||||||||||
| Description |
|
Page API from Norsu pages needs "mgnl:type" get areas. If the definition is configured false for includeSystemProperties, "mgnl:type" cannot be returned.
Workaround: set includeSystemProperties to true and systemProperties should be empty or at least has "mgnl:type"
Dev note: "mgnl:type" is a special property, we should treat it like @id, @path, @name. And it will always be bypassed by the property filter. |
| Comments |
| Comment by Christopher Zimmermann [ 16/Nov/22 ] |
|
I cannot remember - was this different on JCR delivery endpoint? |
| Comment by Canh Nguyen [ 17/Nov/22 ] |
|
There are a lot differences between them. Norsu has nested content while JCR has sub nodes for areas and components for pages. Norsu delivery endpoint has multiple MessageBodyWriter implementations so we implemented property filtering in a wrapper node so that we don't have to re-implement this feature on every MessageBodyWriter. JCR has a complex NodeWrapper system, it unwraps the node to get the type. We didn't implement this pattern to Norsu because it's very complex and difficult to debug. |
| Comment by Oanh Thai Hoang [ 08/Dec/22 ] |
|
Discovery output: mgnl:type with previous code in REST module is considered is system property (since it start with mgnl). So set includeSystemProperties is false will not write nested content (area and component). See code in info.magnolia.pages.domain.AreaContainer#areas
Propose: We always write response for mgnl:type (whether includeSystemProperties is false or true)
Consequence: always write response for nested content in LegacyPageWriter
With following endpoint:
$type: deliveryEndpoint_v2
includeSystemProperties: false
depth: 5
nodeTypeMappings:
pages:
$type: pagesResolver_v2
In case we wanna hide nested content. We can configure additional configuration includeNestedContent is false. See $type: deliveryEndpoint_v2 includeSystemProperties: false includeNestedContent: false depth: 5 nodeTypeMappings: pages: $type: pagesResolver_v2 |
| Comment by Christopher Zimmermann [ 08/Dec/22 ] |
|
Is includeNestedContent just about pages? Does it have relevance anywhere else? What does "depth:5" relate to then. Is depth related to the nested content.... or to sub pages... or is it not used anymore? |
| Comment by Oanh Thai Hoang [ 08/Dec/22 ] |
It can be used without page legacy. Ex: if we configure includeNestedContent is false. $type: deliveryEndpoint_v2 includeSystemProperties: false includeNestedContent: false
No related to nested content. Sorry for confuse. I clone from my yaml |
| Comment by Christopher Zimmermann [ 09/Dec/22 ] |
|
I find it difficult to make a decision. In general a goal is to make the Norsu endpoint work similar to JCR endpoint so that developers have an easy time working with SaaS or DXCORE. This also applies to our sample projects and documentation. Life will be a lot easier for everyone the more similar a SaaS to a DXCORE project is. Re: mgnl:template: Re: includeNestedContent |
| Comment by Oanh Thai Hoang [ 12/Dec/22 ] |
mgnl:type it same meaning like nodetype and do not relate to mgnl:template.
Re: includeNestedContent
Without change any thing. includeSystemProperties false is possible to get just "top" level content of a page. As ticket description "LegacyPageWriter in pages returns empty areas if the endpoint definition not including system properties". There has a use case for return a list of "top" level of pages without nested content is searching page name in find bar. Or using rest end point query full text search (ex: http://localhost:8080/container/.rest/delivery/newnorsu/v1/?q=react-mi) In conclusion. As result of new property is unacceptable, I think includeSystemProperties is enough for now, since childNodetype do not support to filter nested area and component
And So I close ticket as won't fix at the moment. How does that sound? |