Major problems preventing Content Apps from being JCR-agnostic
-
MGNLUI-2662
-
Getting issue details...
STATUS
Action Executor API. We don't debate whether
ActionExecutor
should or should not be responsible for primary action availability checking , but it shouldn't do it _exclusively_ forjavax.jcr.Items
.- Should work over Vaadin Items. Being used in both Browser and Detail sub-apps of Content App it is probably the most severe blocker for the task.
- Availability rules are all affected, but easy to fix, because those that depend on JCR can simply cast to
JcrItemAdapter
/JcrNodeAdapter
. - Still not quite clear what to do with JCR logic inside AbstractActionExecutor itself.
- Serialization/de-serialization of item ids in to strings (basically - URL fragments).
The snippets like the one below are not only JCR-dependent, but also cluttered and hard to understand. What is more - the similar pieces of code are common on all the levels of Content App presenters, sometimes maybe even not once.
URL Fragment to Item Id Transformation in Browser Sub-AppString itemId = null; try { itemId = JcrItemUtil.getItemId(SessionUtil.getNode(workspaceName, path)); // item might have not been found if path doesn't exist if (itemId == null) { itemId = JcrItemUtil.getItemId(SessionUtil.getNode(workspaceName, workbenchRoot)); BrowserLocation newLocation = getCurrentLocation(); newLocation.updateNodePath("/"); getAppContext().updateSubAppLocation(getSubAppContext(), newLocation); } } catch (RepositoryException e) { log.warn("Could not retrieve item at path {} in workspace {}", path, workspaceName); }
Instead we could simply always delegate fragment to id translation (and vice versa) to the component which is specialised on that.
How the same logic should be probably doneObject actualItemId = dataSourceManager.deserializeItemId(urlFragmentPath); // item might have not been found if path doesn't exist if (dataSourceManager.itemExists(actualItemId)) { actualItemId = dataSourceManager.getRootItemId(); BrowserLocation newLocation = getCurrentLocation(); newLocation.updateNodePath("/"); getAppContext().updateSubAppLocation(getSubAppContext(), newLocation); } return actualItemId;
- Mixed signatures of methods in Content App related presenters and events
- Some operate over Vaadin Items, some - over the
JcrItemAdapters
, some, like aforementioned, - over JCR Items themselves. It sometimes leads to ItemId <-> Item conversion several times during delegation.- As a solution - only Item Ids should be used as a transferable during components/apps/sub-apps communication.
- Only workbench knows how to transform Item Id into a Vaadin Item. Ideally - that should be a job of a separate datasource manager component as such functionality is need in various places (e.g. Detail Sub-apps).
- Strings as Item ids everywhere - Object should be used instead (generics would be even better, but that's rather hard to implement with limited support of them in Vaadin Data binding, Dependency Injection etc).
- Some operate over Vaadin Items, some - over the
- JCR-specific terms popping up constantly (workspace, workbench root etc) - can be generalised and/or encapsulated.
- Things that should also be moved out from
BrowserSubApp
in order to make it purely JCR-agnostic (without even child JCR-specific classes)- Application of inline-editing (
BrowserPresenter#editItem
). - Action availability verification (e.g.
BrowserSubAppBase#isSectionVisible
).
- Application of inline-editing (
- Provided the above is resolved -
DetailSubApp
comes JCR-agnostic pretty much for free.- That holds also for most of Dialogs (except for chooser probably, but that's to be researched).
- Field factories are still slightly bound to JCR due to the
BasicTransformer
, but that not the biggest problem - Transformer concept still can be used for cases of other data sources and to some extent custom solution would be needed.
- MGNLUI-2680 - Getting issue details... STATUS info.magnolia.ui.imageprovider.ImageProvider interface is JCR-dependent (could work over Object)
Additional ideas, proposals, API changes etc
- BrowserPresenter#getPreviewImageForId(Object id) - something we probably would want to extract into a separate component in order to be able to alter browser sub-app without extensions etc.
- DataSource management component should probably take over some of the workbench definition like path and other properties that might be need for data querying.
Actions taken
- As it is visible from the diagram below - almost all the components/parts of content apps were using JCR directly or through JCR-Vaadin adapters. We have extracted the DataSource layer which provides interface for common operations required by the content apps.
DataSource layer is represented currently by the
DataSourceManager
interface. As it is shown in the listing below - it is capable of converting item ids to and from strings, fetching Vaadin Items by ids and verifying their existence. It is also capable of providing the Vaadin Containers to the content views/presenters.public interface DataSourceManager { String getItemPath(Object itemId); Object getItemIdFromPath(String strPath); Object getRootItemId(); Item getItem(Object itemId); Container getContainerForViewType(String viewType); boolean itemExists(Object itemId); }
JCR-Agnostic content app artefacts and deliverables
UI: jcr-free-work branch.
DAM: jcr-free-adaptation branch.
FileSystemBrowser module: ssh://git@git.magnolia-cms.com/user/apchelintcev/fs-browser-app.git
Usage Instructions:
- All the affected tests are adapted at least compilation wise, but I suggest -DskipTests flag is used.
- Some checkstyle leftovers might occur, so -Dcheckstyle.ignore=true is also a good idea.
- I suggest using magnolia-empty web-app for testing the concept out as some of the modules (e.g. sitemaps, rss-agg) might need re-build to work correctly.