Wrong dependency info after module uninstall

[alesj]

I've added UndeployOrderClassLoaderUnitTestCase that exposes the next problem (which Scott and me stumbled upon our OSGi demo at JBW):

We have two modules, acme1.jar and acme2.jar.
The number is the version of packages inside (org.jboss.acme.*).
And we have foobar.jar that requires o.j.a.ales#1 (meaning it's from acme1.jar) and o.j.a.scott#2 (respectively from acme2.jar).

And when I uninstall acme2.jar it does unwind foobar's deployment controller context back to Describe.
But invoking DeployerClient.checkComplete produces IncompleteDeploymentException from foobar's added dependency on the acme2.jar instead of giving the right info about o.j.a.scott#2 requirement.

We don't care about the module from where the pckg capabilities come from, so the info is misleading.

Changing RequirementDependencyItem.unresolved to override the AbstractDependencyItem's similar method does the trick since it nullifies IDependOn, hence the info will come from toString which includes requirements.

[adrian.brock]

Two points.

1) Does the same problem exist in the new code? I don't understand your description
about overriding a method with the same name.

2) Does IncompleteDeploymentException use the toHumanReadableString()
method on the DependecyItem?

[alesj]

1) Does the same problem exist in the new code? I don't understand your description
about overriding a method with the same name.

Yup, the problem is still there.
Just run the test case, and tell what is the real missing dependency. :-)

What I meant with overriding is that unresolved from RequirementDI should override the unresolved(Controller) method from AbstractDI, which is what gets called when doing the uninstall.
Probably that's what was already intended in the first place, but we changed the signature of unresolved when introducing cardinality on callbacks.
The method went from (void + no params) to (boolean + Controller param). And RequirementsDI's unresolved is equal to the first/initial usage.

2) Does IncompleteDeploymentException use the toHumanReadableString()
method on the DependecyItem?

Yes.
Once the iDependOn is nullified, this will get used in DeployersImpl.checkControllerContext method.

 Object iDependOn = item.getIDependOn();
 if (iDependOn == null)
 {
 dependency = "<UNKNOWN>";
 actualStateString = "** UNRESOLVED " + item.toHumanReadableString() + " **";
 }

[adrian.brock]

Ok, now I've seen the commit, I understand what you are talking about.
You changed the api in an incompatible way and didn't update the class hierarchy.

There is a similar issue with the demand dependency which is the original
version of depending on an abstract concept and what I copied to create
the RequirementDependencyItem..

I've added @Override to the methods to ensure they are actually overridding
things, but more importantly, I've changed the AbstractDependencyItem
to do what you should have done in the first place:

Compatible change:

 // Old method possibly overridden by subclasses
 public void unresolved()
 {
 if (resolved)
 {
 resolved = false;
 flushJBossObjectCache();
 log.trace("Forced unresolved " + this);
 }
 }

 // New method just calls the old method
 public boolean unresolved(Controller controller)
 {
 unresolved();
 return true;
 }