Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[tck][cm] CMCoordinationTestCase#testCoordinatedConfigurationOnBeforeRegisteredManagedService seems to contain to strict assertions #718

Open
laeubi opened this issue Apr 27, 2024 · 0 comments · May be fixed by #719
Open

[tck][cm] CMCoordinationTestCase#testCoordinatedConfigurationOnBeforeRegisteredManagedService seems to contain to strict assertions #718

laeubi opened this issue Apr 27, 2024 · 0 comments · May be fixed by #719

Comments

@laeubi
Copy link
Contributor

laeubi commented Apr 27, 2024

Currently Equinox fails this Coordinator TCK test-case:

osgi/org.osgi.test.cases.cm/src/org/osgi/test/cases/cm/junit/CMCoordinationTestCase.java

Lines 384 to 441 in 11d440e

public void testCoordinatedConfigurationOnBeforeRegisteredManagedService()
throws Exception {
final List<Boolean> events = new ArrayList<>();
final String pid = this.getClass().getName() + ".mstestpid3";
// add managed service
final Dictionary<String,Object> msProps = new Hashtable<>();
msProps.put(Constants.SERVICE_PID, pid);
this.registerService(ManagedService.class.getName(),
new ManagedService() {
@Override
public void updated(Dictionary<String, ? > properties)
throws ConfigurationException {
events.add(properties != null);
}
}, msProps);
// start a coordination
final Coordinator c = this.getService(Coordinator.class);
final Coordination coord = c.begin("cm-test2", 0);
try {
// create the configuration
final Dictionary<String,Object> props = new Hashtable<>();
props.put("key", "value");
final Configuration conf = this.cm.getConfiguration(pid);
conf.update(props);
sleep();
assertEquals(1, events.size());// update with null because
// registered before coordination
// update configuration
props.put("key2", "value2");
conf.update(props);
sleep();
assertEquals(1, events.size());
// delete configuration
conf.delete();
sleep();
assertEquals(1, events.size());
} finally {
coord.end();
}
// wait and verify listener
sleep();
assertEquals(1, events.size());
assertFalse(events.get(0));
}

the reason is that here is is asserted that exactly one event is triggered:

osgi/org.osgi.test.cases.cm/src/org/osgi/test/cases/cm/junit/CMCoordinationTestCase.java

Line 439 in 11d440e

assertEquals(1, events.size());

because the test-case assumes that the block that is executed under the coordinator results in no event being generated as it creates a configuration and then afterwards delete it.

But the specification says (even though its quite vague):

If configurations are created, updated or deleted and an implicit coordination exists, the Configuration Admin service must delay notifications until the coordination terminates. However the configuration changes must be persisted immediately. Updating a Managed Service or Managed Service Factory and informing asynchronous listeners is delayed until the coordination terminates

What now happens with equinox that the updated method is called twice, once outside the coordination and one after the coordination terminates, because actually something was happening (configuration got deleted) and the "delayed notification at termination" is exactly this.

Given that a ManagedService must be prepared to be called multiple times with null configuration (e.g. when the ConfigurationAdmin is restarted) and the ConfigAdmin should be stateless in regards to what it has offered to the service, I think this is also a valid behavior.

I therefore think the assertion should be that the number of event is not larger than 2 and each value is false.
laeubi added a commit to laeubi/osgi that referenced this issue Apr 27, 2024
@laeubi
More than one event should be allowed in case of coordinated delete
2470657 
Fix osgi#718
This was referenced Apr 27, 2024
Open
Open
laeubi added a commit to laeubi/osgi that referenced this issue May 8, 2024
@laeubi
Require one event should be delivered in case of coordinated delete
e36c6b6 
Fix osgi#718

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
laeubi added a commit to laeubi/osgi that referenced this issue May 9, 2024
@laeubi
Require one event should be delivered in case of coordinated delete
0bc15be 
Fix osgi#718

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

More than one event should be allowed in case of coordinated delete laeubi/osgi
1 participant
@laeubi