Monday, March 15, 2010

Case Study: Add Functionality Without Changes to Code

What I'll describe here is an exercise I've completed that adds functionality to an existing codebase without changing that codebase. The product is not dynamically "pluggable", service-oriented, etc., so I don't have those levers to address the problem; instead, I've used Aspect Oriented Programming (AOP), JSF phase listeners, and the Observer design pattern to add to the codebase in a modular way. This gives me an easy way to remove that code if that's called for - and, for this exercise, that's the primary goal.


Our team's product cycle is transitioning with one release in test cycle, moving soon to a Beta audience; and with some new functionality targeted for the next release. These releases are being managed in separate SCM trees; however, due to various infrastructure limitations, we're being asked to wait on checking in changes to the test-cycle branch. But I don't want this to stop me from moving forward on that release.

There are various well-known problems, of course, if one makes "too many" changes to a  codebase before checking things in:
  1. There could be a conflict-resolution exercise between my changes and my teammates' changes to the same files.
  2. If a bug is found in Beta, that code will need a bugfix and a patch - but if I've intermingled new stuff in the code needing bugfixes, I have a potentially error-prone, tedious and time-consuming version control exercise.
There are SCM strategies to address #2; but for the sake of argument, let's just say that my constraint is to develop the new functionality with minimal - if not zero - impact to existing code, to avoid both potential problems from above.

For that matter, I wanted to challenge myself to see just how much I could do with this type of constraint - so I gave myself some "rules":
  1. keep the code changes as modular as possible so they can easily be added, for that matter easily removed if called for;
  2. ideally, no changes at all are made to existing code;
  3. if for some reason, I must change existing code, it must be done in such a way that new classes, declarations, etc. can be introduced, but existing code must not make reference to the new artifacts
So, e.g., I can use aspects, includable XML files, listeners of various flavors, and the like. Looking more closely at my "rules", it appears that #2 and #3 actually are the "how-to" supporting the "what" of #1 - i.e., the true goal here is to make it easy to add or remove the functionality if called for.

Here's the new functionality to be added:
  1. At various points during the customer's use of the product, a certain process should be launched to accomplish a particular goal. I'm intentionally being vague since the details don't really matter here.
  2. For each version of the product, that process should be executed just once without any confirmation from the user that it should proceed; but once it's been executed that first time (for a particular product release), the user should be issued some kind of popup confirmation dialog (this is a webapp UI) that offers a choice: allow the process to proceed with due caution, or just bail out.
  3. Every time a new product release is installed, the warning mechanism resets, i.e. the very next time the user launches the process, no such confirmation dialog appears.
The relevant pieces of the technology stack that I'll be "modifying" includes:
  1. A JSF-based web tier, using in particular the IceFaces component set;
  2. Spring 3.0
Breaking things down, it seems I'll need to do things like this:
  1. Persist some information around the product version, to be generated at build time and deployed with product installation;
  2. intercept the rendering of the UI button that launches the process so that the confirmation dialog can be associated with it, such that when the button is clicked, the confirmation will appear (giving the user a choice of proceeding or not) - but the confirmation appears only if the process has already executed for this product release;
  3. persist information after each process execution indicating this execution has occurred for a given product installations; and
  4. interpret the lack of that persisted information to indicate the process has not executed for this installation. This will be the initial condition, and the condition after each new product release is installed.

What I did to address #1 is trivial, and frankly out of scope for the purposes of this post. Likewise, #4 is reasonably straightforward; no additional details are needed around this.

To address #2 - intercept the rendering of the user action - my initial instinct tells me this sounds like an aspect...well, to be honest, I was looking for some excuses to use aspects - not only because I think this is a good approach to adding functionality in a modular way, but because I want to get better at AOP. A solution for #3 might also use an aspect, so that after the process has been run, the process-execution-history is updated.

As it turned out, the most interesting problem was intercepting the rendering of the UI button to attach a confirmation dialog dynamically. Note that sometimes the confirmation should be there - if the process has already been run - and sometimes, the process should just get launched without any warning to the user.

Let's first establish the confirmation dialog to be used. I'm using IceFaces, which provides a panelConfirmation tag that I use like this:

    <!--
    Warn user if process has already executed for this release
    -->
    <ice:panelConfirmation 
       id="warnProcessExecuted" 
       message="#{msgs['confirm.rerun']}"
       acceptLabel="Yes" cancelLabel="No"/>

The straightforward way to use this is to simply add it as an attribute to the existing IceFaces button that launches the process:

<ice:commandButton id="launchProcess"
     value="Launch Process"
     immediate="true"
     confirmationPanel="warnProcessExecuted"
     actionListener="#{eventsManager.startProcess}"/>

But this violates my self-imposed constraint described by rule #2 above: "ideally, no changes at all are made to existing code".  So my initial thought is to do this:
  1. intercept the handling of the action after user clicks on the button;
  2. find the JSF component associated with the button; and
  3. dynamically add the confirmationPanel attribute to that button component.
Doing things this way also would factor out the conditional part: if the process has not executed yet, there's no need to add the confirmation. But it turns out that trying to add the attribute during the action handling is too late - what I want is to intercept the rendering of the button. This sounds like I could use a JSF phase listener that listens for the beginning of the RENDER_RESPONSE phase to do this. This worked out just fine, but the thing about it that bothered me was that this happens several times for each action, and the method that I use to attach the attribute gets called each time. In this case, no harm is done; but I'd like to find a more general solution so my code execution is more deterministic and less at the mercy of the JSF lifecycle - bottom line, so that the method of interest is called only once at the beginning of the RENDER_RESPONSE phase. Here's how I did it:

First I create a reusable hierarchy of JSF phase listeners. The superclass provides the PhaseListener implementation, and additionally provides a registration mechanism - with which it remembers any interested listeners for a given phase. Each registered listener implements a PhaseObserver interface, and is notified at the beginning and end of each JSF phase:

public class PhaseMonitor implements PhaseListener {

    private static Map<JsfPhaseId, Set<PhaseObserver>> observers =
        new HashMap<JsfPhaseId, Set<PhaseObserver>>();

    // notify all registered observers for this phase that the phase has begun
    public void beforePhase(PhaseEvent event) {

        JsfPhaseId phaseId = JsfPhaseId.getJsfPhaseId(event.getPhaseId());
        Set<PhaseObserver> these = observers.get(phaseId);
        if (these != null) {
            for (PhaseObserver observer : these) {
                observer.notifyBeforePhase(phaseId);
            }
        }
    }

    // notify all registered observers for this phase that the phase has ended
    public void afterPhase(PhaseEvent event) {

        JsfPhaseId phaseId = JsfPhaseId.getJsfPhaseId(event.getPhaseId());
        Set<PhaseObserver> these = observers.get(phaseId);
        if (these != null) {
            for (PhaseObserver observer : these) {
                observer.notifyAfterPhase(phaseId);
            }
        }
    }

    // observer pattern: interested observers implement the PhaseObserver interface and register their interest
    public static void register(JsfPhaseId phase, PhaseObserver observer) {

        Set<PhaseObserver> these = observers.get(phase);
        if (these == null) {
            these = new HashSet<PhaseObserver>();
        }
        these.add(observer);
        observers.put(phase, these);
    }

    protected PhaseId getMyPhaseId() { // subclasses will override this for each JSF phase
        return null;
    }

    public PhaseId getPhaseId() {
        return getMyPhaseId();
    }
}


The JsfPhaseId is a first-class enum, provided not only so the observers can reference a simple enum constant (which the JSF PhaseID does not provide - it is not an enum (!!)), but so that client code need not be tightly coupled to JSF. Granted, clients will use the JsfPhaseId, which uses JSF, so the deployment will be coupled to JSF - but at least I've encapsulated my usage of JSF by providing this facade:

public enum JsfPhaseId
{
    APPLY_REQUEST_VALUES, INVOKE_APPLICATION, PROCESS_VALIDATIONS,
    RENDER_RESPONSE, UPDATE_MODEL_VALUES, RESTORE_VIEW, UNKNOWN;

    public static JsfPhaseId getJsfPhaseId(PhaseId phaseId) {

        if (phaseId.equals(PhaseId.APPLY_REQUEST_VALUES)) {
            return JsfPhaseId.APPLY_REQUEST_VALUES;
        } else if (phaseId.equals(PhaseId.INVOKE_APPLICATION))  {
            return JsfPhaseId.INVOKE_APPLICATION;
        } else if (phaseId.equals(PhaseId.PROCESS_VALIDATIONS)) {
            return JsfPhaseId.PROCESS_VALIDATIONS;
        } else if (phaseId.equals(PhaseId.RENDER_RESPONSE)) {
            return JsfPhaseId.RENDER_RESPONSE;
        } else if (phaseId.equals(PhaseId.UPDATE_MODEL_VALUES)) {
            return JsfPhaseId.UPDATE_MODEL_VALUES;
        } else if (phaseId.equals(PhaseId.RESTORE_VIEW)) {
            return JsfPhaseId.RESTORE_VIEW;
        }
        return JsfPhaseId.UNKNOWN;
    }
}

Each phase-specific subclass of PhaseMonitor does this:

public class RenderResponsePhaseMonitor extends PhaseMonitor {

    private PhaseId phaseId = PhaseId.RENDER_RESPONSE;
    protected PhaseId getMyPhaseId() {
        return phaseId;
    }
}

This is repeated for the other JSF phases. Now I need to add these phase listeners to the code, but in a way that minimizes impact that existing code. So, I do not want to modify my existing JSF configuration file by declaring these listeners; instead, I add a new config file using the web-tier deployment descriptor (web.xml):

<context-param>
        <param-name>javax.faces.CONFIG_FILES</param-name>
        <param-value>/WEB-INF/faces-config-application.xml, /WEB-INF/faces-config-listeners.xml</param-value>
</context-param>

That config file looks like this:

<faces-config xmlns="http://java.sun.com/JSF/Configuration">
    <!-- listen for phase events to facilitate phase listening, notification -->
    <lifecycle>
        <phase-listener>com.mybiz.web.jsf.lifecycle.ApplyRequestValuesPhaseMonitor</phase-listener>
        <phase-listener>com.mybiz.web.jsf.lifecycle.InvokeApplicationPhaseMonitor</phase-listener>
        <phase-listener>com.mybiz.web.jsf.lifecycle.ProcessValidationsPhaseMonitor</phase-listener>
        <phase-listener>com.mybiz.web.jsf.lifecycle.RenderResponsePhaseMonitor</phase-listener>
        <phase-listener>com.mybiz.web.jsf.lifecycle.RestoreViewPhaseMonitor</phase-listener>
        <phase-listener>com.mybiz.web.jsf.lifecycle.UpdateModelValuesPhaseMonitor</phase-listener>
    </lifecycle>
</faces-config>

The interested observers implement this interface:

public interface PhaseObserver {

    public void notifyBeforePhase(JsfPhaseId phaseId);
    public void notifyAfterPhase(JsfPhaseId phaseId);
}

With these levers in place, I can now implement an observer that will be notified once and only once at the beginning of the RENDER_RESPONSE phase, at which time it will determine if a confirmation for the user is needed; if so, it will find the JSF component of interest and attach the confirmation panel, and, if the user elects to proceed, updating some persistent history "somewhere" with information about this execution; else it sets the attribute value for the confirmation panel to an empty string so that no such dialog pops up. Since I don't want to minimize my changes to existing code, I add this functionality with an aspect:

@Aspect
public class ExecutionAspects implements PhaseObserver {

    public ExecutionAspects() {
        PhaseMonitor.register(JsfPhaseId.RENDER_RESPONSE, this);
    }

    private ExecutionHelper executionHelper;  // injected via Spring
    public void setExecutionHelper(ExecutionHelper theHelper) {
        executionHelper = theHelper;
    }

    /**
     * Establish a pointcut that describes a method which we know will be called when the button is rendered
     */
    @Pointcut("execution(* com.mybiz.view.ViewHelper.isLaunchProcessButtonShowing(..))")
    public void isLaunchProcessButtonShowing() {
    }

    // here's how we enforce the "once and only once" constraint:
    private boolean needConfirmationForThisRequest = true;
    public void notifyBeforePhase(JsfPhaseId phaseid) {
        needConfirmationForThisRequest = true;
    }
    public void notifyAfterPhase(JsfPhaseId phaseid)
    {
        needConfirmationForThisRequest = false;
    }

    /**
     * Intercept rendering of button to detect whether or not the process has already been run for this release
     */
    @Around("isLaunchProcessButtonShowing()")
    private boolean interceptRender(ProceedingJoinPoint pjp) throws Throwable {
        if (needConfirmationForThisRequest)
        {
            // get UI component, add confirmation panel if needed
            HtmlCommandButton button =
                    (HtmlCommandButton) FacesUtils.findComponent(FacesUtils.getFacesContext().getViewRoot(),
                            "launchProcess");  // search from root of JSF component tree for the launch button ID
            if (executionHelper.getProcessHasExecutedForThisProduct()) {
                // attach the confirmation
                button.setPanelConfirmation("warnProcessExecuted");
            } else {
                // set the confirmation attribute to an empty string so no warning will appear
                button.setPanelConfirmation("");
            }
            needConfirmationForThisRequest = false;
        }
        // return value as normal
        return (Boolean)pjp.proceed();
    }

    /**
     * Establish a pointcut that intercepts the user action of launching the process
     */
    @Pointcut("execution(* com.mybiz.view.EventsHelper.startProcess(..))")
    public void startProcess()
    {
    }

    /**
     * Intercept launching of process to facilitate updating the execution history after it's done
     */
    @Around("startProcess()")
    private void interceptProcess(ProceedingJoinPoint pjp) throws Throwable
    {
        pjp.proceed();
        // update history so user can be warned that it's already been run (next time it's requested)
        executionHelper.updateConfMergeHistory();
    }
}

Now the aspect needs to be added to runtime execution; I already have a Spring context file for the existing product functionality, but again I don't want to change that file. Instead, I modify the web-tier deployment descriptor to add a new one:

<context-param>
    <param-name>contextConfigLocation</param-name>
    <param-value>/WEB-INF/beans-all.xml, /WEB-INF/beans-execution.xml</param-value>
</context-param>

That Spring context file manages the ExecutionAspects class, supplying the ExecutionHelper dependency:

<beans xmlns="http://www.springframework.org/schema/beans"
       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
       xmlns:aop="http://www.springframework.org/schema/aop"
       xsi:schemaLocation="
       http://www.springframework.org/schema/beans
       http://www.springframework.org/schema/beans/spring-beans.xsd
       http://www.springframework.org/schema/aop
       http://www.springframework.org/schema/aop/spring-aop-3.0.xsd
       ">

    <!--
    Configuration file used to maintain execution information - i.e. as a control for when the process
    is run with or without warning, we need to maintain information about what release is this
    product, and compare that to what release was the last execution run against.
    -->
    <bean id="executionHelper" scope="session" class="com.mybiz.aspects.ExecutionHelper">
    </bean>

    <aop:aspectj-autoproxy proxy-target-class="false"/>

    <bean id="execution-aspect" class="com.mybiz.aspects.ExecutionAspects">
        <property name="executionHelper" ref="executionHelper"/>
    </bean>

</beans>

The ExecutionHelper is of minimal interest here; it simply does CRUD on "some persistence mechanism" to manage information about what is the current product release and what particular release has the process last been executed against. It provides a convenience method (getProcessHasExecutedForThisProduct(), as seen above) encapsulating all of that information with a boolean indicating exactly what it's name suggests.


So let's revisit my initial goals and see how well I've done:
  1. keep the code changes as modular as possible so they can easily be removed if called for;
  2. ideally, no changes at all are made to existing code;
  3. if for some reason, I must change existing code, it must be done in such a way that new classes, declarations, etc. can be introduced, but existing code must not make reference to the new artifacts (this facilitates rule #1)
#1: are my changes modular? In the sense that they can be removed very easily if needed, yes - here's what I'd need to do to remove this functionality: change this -->

<param-value>/WEB-INF/beans-all.xml, /WEB-INF/beans-execution.xml</param-value>

to this -->

<param-value>/WEB-INF/beans-all.xml</param-value>nclude src="/WEB-INF/includes/panel-confirmation.jspx"/>

Without the single additional declaration of Spring context that attaches the new aspect to the runtime, the aspect will not get instantiated, let alone executed. All of the existing code - well, most of it anyway - remains untouched. The new code can be left as an addition to the codebase, ready for activation whenever product management calls for that.

You'll notice I said most of the existing code is unchanged. Beyond the addition of the 2nd Spring context, as noted here, and the additional JSF Config file that declares the hierarchy of phase listeners, I did need to make one change to the JSF file that declares the command button; I needed to include the new JSF file that declares the confirmation within the same form. Apparently this is a constraint of the component set, and it is not surprising. So I ended up doing this:


<ice:commandButton id="launchProcess"
    value="Launch Process"
     immediate="true"
    actionListener="#{eventsManager.startProcess}"/>
             
 <!-- confirmation panel must be in same form as the button that references it. The
  "launchProcess" button, above, is managed dynamically to point to this confirmation
  in an aspect. For better modularity, include the panelConfirmation snippet:
  -->
  <ui:include src="/WEB-INF/includes/panel-confirmation.jspx"/>

So I didn't quite succeed with rule #2 -- I did have to make some changes to existing code. But for all intents and purposes, it's not an issue -- since the change to the code is a declaration that is not referenced by existing code. I'd argue likewise for the addition of the JSF Config file with the hierarchy of phase listeners and built-in observer mechanism - this is, in my opinion, a worthwhile addition to any JSF application, one that I'll likely continue doing from this point forward. I just happened to leverage it with an aspect that registers as an interested observer of the RENDER_RESPONSE phase. That is the essence of modularity.

Again, not quite successful with rule #3 - since the changes to the web.xml do make reference to the new JSF Config and Spring context files. But again, in spirit that goal was only in place to facilitate success on rule #1 - and bottom line, I can add or remove this new functionality without breaking a sweat.

No comments:

Post a Comment