[MAGNOLIA-2722] Make DefaultContent implement Map directly Created: 07/May/09 Updated: 19/Jan/10 Resolved: 19/Jan/10 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | core |
| Affects Version/s: | 4.0 |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major |
| Reporter: | Fabrizio Giustina | Assignee: | Fabrizio Giustina |
| Resolution: | Won't Fix | 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)
|
| Date of First Response: |
| Description |
|
now that we heavily rely on NodeMapWrappers for templating it could be better if we move the implementation of the Map interface directly to DefaultContent. WTYT of this move in 4.1? |
| Comments |
| Comment by Magnolia International [ 15/Jun/09 ] |
|
We don't really see the benefit of this (added confusion, api complex enough as it is, error-prone (think properties with same name as a child name, etc) – do you need such wrapping elsewhere than in jsp template renderers ? The wrappers currently should already be wrapping parents and children correctly – maybe there's a bug in the NodeMapWrapper ? |
| Comment by Magnolia International [ 10/Aug/09 ] |
|
Any feedback or comment? |
| Comment by Fabrizio Giustina [ 12/Aug/09 ] |
well, the idea is to implement Map in DefaultContent, not exposing any additional method in the Content interface. APIs will not change, defaultContent is never used explicitely. The point here is that we already wrap a jcr node in a magnolia Content. That was initially done with the idea of creating objects that were easier to use also directly in templates. Then we realized that exposing properties in a different way (Map) was far better for jstl and freemarker and added another level of wrapping. I feel that it may be simpler and less confusing to just add the needed methods in Content and remove the NodeMapWrapper level. Also think about jcr queries that returns lists of Content objects: if you want to use them in templates you have to wrap them manually (or just use them in a different way than the predefined objects like ${content}... but that's pretty confusing....) |
| Comment by Philipp Bärfuss [ 19/Jan/10 ] |
|
I prefer to keep the core classes as clean as possible (see |