[MGNLUI-606] info.magnolia.ui.vaadin.integration.jcr whole package need review Created: 24/Jan/13 Updated: 10/Mar/21 Resolved: 10/Mar/21 |
|
| Status: | Closed |
| Project: | Magnolia UI |
| Component/s: | framework |
| Affects Version/s: | 5.0 |
| Fix Version/s: | None |
| Type: | Task | Priority: | Neutral |
| Reporter: | Espen Jervidalo | Assignee: | Unassigned |
| Resolution: | Obsolete | Votes: | 0 |
| Labels: | adapters | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Template: |
|
||||||||
| Acceptance criteria: |
Empty
|
||||||||
| Task DoR: |
Empty
|
||||||||
| Date of First Response: | |||||||||
| Description |
|
It's an abstract mess in general grown over time. This could be simplified a lot by simple refactorings, documentation and probably get rid of some classes. all those methods in there take a node or an Id to perform actions, but the the classes are wrapping/adapting a single property or node, so you could theoretically create an adapter and pass a not relating node into it. Why pass those references around? —
info.magnolia.ui.vaadin.integration.jcr.AbstractJcrAdapter#changedProperties — info.magnolia.ui.vaadin.integration.jcr.AbstractJcrNodeAdapter#getItemProperty — — — — A marginal remark for (Entry<String, Property> entry : changedProperties.entrySet()) { ... } ATM, removing properties will clear removedProperties HashMap, however, applying changes to (saving) properties doesn't remove properties from the changedProperties HashMap. |
| Comments |
| Comment by Daniel Lipp [ 05/Feb/13 ] |
|
re "From code I actually think it's safe to not synchronize there. Those adapters are pretty transient and basically created on the fly so IMHO the only situation where an instance is used twice is, when editing an item of a table in a Dialog. As dialogs are modal to the table this should still be safe. I cannot think of any situation where different users would end up with the same instance of such an adapter. |