Uninstall OnDemand context if there are no more dependencies

[kabirkhan]

I have knocked up a prototype for https://jira.jboss.org/jira/browse/JBKERNEL-4. A few of the existing tests fail since the order of uninstall has changed. What I am doing is:

 protected void uninstallContext(ControllerContext context, boolean trace)
 {
 //Existing code

 DependencyInfo dependencies = context.getDependencyInfo();
 if (dependencies != null)
 {
 Set<DependencyItem> iDependOn = dependencies.getIDependOn(null);
 if (iDependOn.isEmpty() == false)
 {
 for (DependencyItem item : iDependOn)
 {
 if (item.isResolved()) //TODO Is this check necessary
 {
 Object name = item.getIDependOn();
 ControllerContext other = getContext(name, null);
 if (other == null)
 {
 log.warn("Could not find dependency while uninstalling on demand contexts for " + item);
 continue;
 }
 if (other.getMode() != ControllerMode.ON_DEMAND)
 continue;

 DependencyInfo otherDependencies = other.getDependencyInfo();
 if (otherDependencies == null)
 continue;

 Set<DependencyItem> dependsOnOther = otherDependencies.getDependsOnMe(null);
 boolean isRequired = false;
 for (DependencyItem dependsOnOtherItem : dependsOnOther)
 {
 ControllerContext dependsContext = getContext(item.getName(), null);
 if (dependsContext == null)
 {
 log.warn("Could not find dependency while uninstalling on demand contexts for " + item);
 continue;
 }

 int requiredIndex = states.indexOf(item.getWhenRequired());
 int actualIndex = states.indexOf(dependsContext.getState());
 if (requiredIndex <= actualIndex)
 {
 isRequired = true;
 break;
 }
 }
 if (!isRequired)
 {
 //For some reason uninstallContext() uninstalls to the state below the passed in one, add one
 int index = states.indexOf(ControllerMode.ON_DEMAND.getRequiredState());
 index++;
 ControllerState state = states.get(index);
 uninstallContext(other, state, trace);
 }
 }
 }
 }
 }
 }

I'll look into the failing tests and fix those, which might mean needing to change the tests themselves in trunk. The JIRA issue mentions that the uninstall of the OnDemand context should happen asynchronously, which will complicate things, but will be possible if a real requirement, I'll try to fix the tests first though.

[alesj]

 int requiredIndex = states.indexOf(item.getWhenRequired());
 int actualIndex = states.indexOf(dependsContext.getState());
 if (requiredIndex <= actualIndex)

Such code should not exist, as it depends on the impl details (== List) of the states model.

You should do this state checks against ControllerStateModel.
e.g. for tree states the model will change, but the rest of the code won't have to be fixed/modified ;-)
(at least not in a hard to detect/find it way)

[kabirkhan]

Sure, I can change it. There are several other examples of this though, or are they subtly different?

 protected void change(ControllerContext context, ControllerState state, boolean trace) throws Throwable
 {
 int currentIndex = states.indexOf(fromState);
 int requiredIndex = states.indexOf(state);
 ...
 while (currentIndex > requiredIndex)
 {
 uninstallContext(context, trace);
 currentIndex = states.indexOf(context.getState());
 }
 }

 protected void uninstallContext(ControllerContext context, ControllerState toState, boolean trace)
 {
 int targetState = states.indexOf(toState);
 if (targetState == -1)
 log.error("INTERNAL ERROR: unknown state " + toState + " states=" + states, new Exception("STACKTRACE"));

 while (true)
 {
 ...
 if (targetState > currentState)
 return;
 else
 uninstallContext(context, trace);
 }
 }

[alesj]

Sure, I can change it. There are several other examples of this though, or are they subtly different?

Ah, you can call those my laziness :-),
as I seem to miss them when I was porting states checking to CSM.

You can fix that, or I will eventually.

[kabirkhan]

"alesj" wrote:

Such code should not exist, as it depends on the impl details (== List) of the states model.

I am leaving this as is for the moment until JBKERNEL-4 and JBKERNEL-25 are complete. JIRA to use ControllerStateModel https://jira.jboss.org/jira/browse/JBKERNEL-48

[kabirkhan]

Looking at http://community.jboss.org/message/522801#522801 I discovered that the uninstall is checking the wrong thing.

When iterating over dependsOnMe instead of calling getContext() which checks the current controller and the parent for the reverse dependency, we should be calling getContexts(Object name, Set<ControllerContext) which checks the current controller and children, since the reverse dependencies can not come from a parent.

https://jira.jboss.org/jira/browse/JBKERNEL-85