[MAGNOLIA-6898] Renderer's configured type not taken into account – node name is current "type" Created: 07/Dec/16  Updated: 23/Dec/16  Resolved: 22/Dec/16

Status: Closed
Project: Magnolia
Component/s: rendering
Affects Version/s: 5.4
Fix Version/s: 5.5.1

Type: Bug Priority: Neutral
Reporter: Philip Mundt Assignee: Roman Kovařík
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

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: Definitions app problems
Sprint: Kromeriz 75
Story Points: 3

 Description   

With the adaption of the rendering module to the new registry API (see MAGNOLIA-6070) and the deprecation of info.magnolia.rendering.renderer.registry.ConfiguredRendererProvider, the configured type of any renderer is not taken into account anymore.

Furthermore getting any renderer by method info.magnolia.rendering.renderer.registry.RendererRegistry#getRenderer(String type) will not return the renderer by its type but rather by configured name = it's node name.

This problem was revealed by problem reporting of node2bean and the definitions app. E.g for renderer jsp from /modules/rendering/renderers/jsp:

Property [type] not found in class [info.magnolia.rendering.renderer.JspRenderer], property is not assigned

Suggested solution (tbd)

"Add property type" to interface info.magnolia.rendering.renderer.Renderer and adjust RendererRegistry to be capable of retrieving renderers by their type.

Notice

Luckily this issue might not be very severe, as it was relatively common to have concurrence in node name and type.

Affected renderers

  • site and site-jsp (module: site)
  • sitemap (module: google-sitemap)
  • textResource, referenceResource and binaryResource (module: resources)
  • noscript and plaintext (module: templating)
  • jsp and freemarker (module: rendering)
  • possibly more


 Comments   
Comment by Roman Kovařík [ 16/Dec/16 ]

The render type has probably never worked as a property. The node name was used since 4.5, see https://git.magnolia-cms.com/projects/PLATFORM/repos/main/browse/magnolia-rendering/src/main/java/info/magnolia/rendering/renderer/registry/ConfiguredRendererManager.java?at=refs%2Ftags%2Fmagnolia-4.5#112

Consider the effort to introduce such thing:

  1. Add getter/setter to AbstractRenderer
  2. Implement custom info.magnolia.config.registry.DefinitionMetadataBuilder
  3. Implement custom info.magnolia.config.registry.DefinitionMetadataBuilder.DefinitionMetadataImpl to hold the render type.
  4. As the builder is created from info.magnolia.config.registry.DefinitionReference which doesn't have such information, we would need to create a custom Jcr/YamlConfigurationFactory/Builder which is really silly.
    Another option is to override info.magnolia.rendering.renderer.registry.RendererRegistry#register(info.magnolia.config.registry.DefinitionProvider<info.magnolia.rendering.renderer.Renderer>) which would change the metadata to include the render type from the render...which is kind of a hack...

It's really pity that our registry looks flexible enough to provide a custom metadata builder but that's not true in the end...
cc pmundt

We can also go with a simple solution and iterate the renderers on getProvider() but I don't see a reason why to do it if nobody has ever needed it.

Comment by Jan Haderka [ 20/Dec/16 ]

Arch meeting: yeah, let's go ahead and remove this property. It is indeed obsolete, not used for number of years and we don't see short term need to bring it back and make it work.

Upon finishing up this ticket, docu review will be necessary to make sure we don't mention type property anywhere and explain that rendererType refers to actual renderer name.

Generated at Mon Feb 12 04:18:53 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.