You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 29 Next »

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_ for javax.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-App
      String 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 done
      Object 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).
  • 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). 
  • 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. The naming of the interface is probably not the best one, alternative suggestion - DataSourceService or simply DataSource.

    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);
}

    The additional features that are/might be required by sub-apps are represented by additional interfaces (currently there're two of them - SupportsVersions and SupportsCreation)

  • Currently DataSource is configured per sub-app (JCRDatasource is default). Ideally it should be injectable right away, but currently it is fetched from provider class. Proposal: we create data source on app/sub-app ComponentProvider creation.
  • DataSource provides Vaadin containers for content views and items for actions and detail sub-apps. Even though the ids are shared and must have the same meaning for the both cases - the actual Vaadin Items stored in container and standalone ones can be different. E.g. for FileSystemDataSource use case items displayed in tables simply carry the meta info of the file, whereas the one used in DetailSubApp also provide access to the binary data of the file and allow to modify it through Vaadin property. However, Item Id is the same for both cases - java.io.File.

  • ImageProvider interface now looks like this:

    public interface ImageProvider {
    String getPortraitPath(Object itemId);
    String getThumbnailPath(Object itemId);
    String resolveIconClassName(String mimeType);
    Object getThumbnailResource(Object itemId, String generator);
}

  • All the events that were carrying JCR items now carry inly Object item ids. E.g. SelectionChangedEvent:

    public class SelectionChangedEvent implements Event<SelectionChangedEvent.Handler> {
    private final Set<Object> itemsIds;
    public SelectionChangedEvent(Set<Object> itemIds) {
        this.itemsIds = itemIds;
    }

    public Object getFirstItemId() {
        if (itemsIds != null && !itemsIds.isEmpty()) {
            return itemsIds.iterator().next();
        }
        return null;
    }
    public Set<Object> getItemIds() {
        return itemsIds;
    }
}
  • Things still to cover
    • Thumbnail view
    • Inline editing
    • Default action availability
    • Container configuration.

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.
  • No labels