I recently needed to review project code in order to validate if it makes problems when upgrading from AEM 5.6 to AEM 6.x; so my focus wasn’t on the code in the first place, but on some other usual suspects (JCR queries etc). But having seen a few dozen classes I found a pattern, which I then found all over the code: the excessive use of String operations. With a special focus on string operations on repository paths.
For example something like this:
String[] segments = resource.getPath().split("/");
String settingsPath = "/" + StringUtils.join(segments,"/",0,2) + "/settings/jcr:content";
Resource settings = resourceResolver.get(settingsPath);
ValueMap vm = settings.adaptTo(ValueMap.class);
String language = vm.get("language");
(to read settings, which are stored in a dedicated page per site).
Typically it’s a weird mixture of String methods, the use of StringUtils classes plus some custom helpers, which do things with ResourceResolvers, Sessions and paths. Spread all over the codebase. Ah, and it lacks a lot of error checking (what if the settings page doesn’t contain the “language” property? adaptTo() can return “null”).
Sadly that problem not limited to this specific project code, I found it in many other projects as well.
Such a code structure is a clear sign for the lack of abstraction and guidance. There are no concepts available, which eliminate the need to operate on strings, but the developer is left with the only abstraction he has: The repository and CRXDE Lite’s view on it. He logs into the repository, looks at the structure and then decides how to mangle known pieces of information to get hold of the things he needs to access. If there’s noone which reviews the code and rejects such pieces, the pattern emerges and you can find it all over the codebase. Even if developers create utility classes for (normally every developer creates one on its own), it’s a bad approach, because these concepts are not designed (“just read the language from the settings page!”), but the design “just happens“; there is no focus on it, therefor quality is not enforced and error handling typically quite poor.
Another huge drawback of this approach: It’s very hard to change the content structure, because at many levels assumptions about the content structure are used, which are often not directly visible. For the example the constants “0” and “2” in the above code snippets determine the path prefix, but you cannot search for such definitions (even if they are defined as constant values).
If the proper abstraction would be provided, the above mentioned code could look like this:
String language = "en";
Settings settings = resource.adaptTo(Settings.class);
If (settings != null) {
language = settings.getLanguage();
}
This approach is totally agnostic of paths and the fact that settings are stored inside a page. It hides this behind a well-defined abstraction. And if you ever need to change the content structure you know where you need to change your code: Only in the AdapterFactory.
So whenever you see code which uses String operations on paths: Rethink it. Twice. And then come up with some abstraction layer and refactor it. It will have a huge impact on the maintainability of your code base.
Things on a content management system 
I think your last code sample is missing the important bit where it actually gets hold of the Resource.
More generally, where do we go to learn all this? This blog?
Hi Carey,
this blog is not really suited as a starting point to learn AEM 🙂 I see https://docs.adobe.com/docs/en/aem/6-2/develop/the-basics.html as a good starting point, but you should probably also attend an AEM training as well (the nice thing is that there you get a developer license for AEM).
And regarding your question where to get the “resource” from: Typically you get it from the request or a requestresolver. For example:
SlingHttpServletRequest request = ...
resource = request.getResource();
or
path="/content/foobar/mysite";
ResourceResolver resourceResolver = ...;
Resource resource = resourceResolver.getResource (path);
Sorry, I should have been clearer about “all this”. We’ve attended Adobe training at my employer, but we still end up writing things like:
getPageManager().getPage(getCurrentPage().getAbsolutePath(2) + “/services”)
and I don’t know how I would do that without concatenating strings to build the path.
Hi,
ok, I understand. Looks like that
getCurrentPage().getAbsolutePath(2)has a special meaning in your application. Is it the root of your site? Anyways, introduce a proper abstraction, likeSite. Then you need to make sure, that you can access thisSiteabstraction from all locations, without to know that it is located on level 2. For that I would implement a AdapterFactory, which hides this knowledge from its users. So you can use something like this:Site mySite = currentPage.adaptTo(Site.class);
And then you implement methods on this Site abstraction, which cover your specific usecases. For example
public Paget getRootPage();
which returns the RootPage of your Site. Now it’s about the meaning of this special page “services” (there must be a reason why you specifically want this page and not one of it’s siblings). And so on.
If you use this approach, you have to implement the knowledge about the content structure at exactly 1 place. For that you can use string operations, operating on the resource tree, whatever. In the end it’s important that this implementation is hidden behind an abstraction, and that everyone else is using this abstraction. So you don’t end up with hardcoding this appraoch all over your codebase. It’s much easiert to test and much easier to change if there’s the need to.