[
https://issues.apache.org/jira/browse/CAMEL-9313?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15013567#comment-15013567
]
Grzegorz Grzybek commented on CAMEL-9313:
-----------------------------------------
Thanks for the analysis [~asiepert].
You're right - this has changed after 2.15.2. And it's true that if you put a
breakpoint (to hold "main" thread) in {{createBundleContext()}}, it fails in
2.15.2 too.
The reason is [this
line|https://github.com/apache/camel/blob/camel-2.15.3/components/camel-test-blueprint/src/main/java/org/apache/camel/test/blueprint/CamelBlueprintTestSupport.java#L143].
The changes I've made are result of very unpredictable results of
camel-test-blueprint tests in CI.
To describe the problem, be aware that there are 3 groups of threads involved:
# {{main}} thread, where test is run - this is started by JUnit,
maven-surefire-plugin, etc.
# threads related to aries blueprint
# threads related to felix configadmin (sometimes even started with {{new
Thread(...).start()}}.
My ultimate goal was to achieve (at least) 100% reliability of the test - to
eliminate all race conditions. And the only way to do it is to do some kind of
synchronization - using {{java.util.concurrent}} primitives and listeners to BP
/ configadmin events.
So, when you add {{trace="{{tracing}}"}} to {{<camelContext>}}, then in 2.15.2
there's great chance (unless you put a breakpoint to hold {{main}} thread),
that BP container will get initialized *after*
[this|https://github.com/apache/camel/blob/camel-2.15.3/components/camel-test-blueprint/src/main/java/org/apache/camel/test/blueprint/CamelBlueprintTestSupport.java#L145-L150]:
{code:java}
if (file != null) {
if (!new File(file[0]).exists()) {
throw new IllegalArgumentException("The provided file \"" + file[0] + "\"
from loadConfigAdminConfigurationFile doesn't exist");
}
CamelBlueprintHelper.setPersistentFileForConfigAdmin(answer, file[1],
file[0], props, symbolicName, bpEvents, expectReload);
}
{code}
This leads to invocation of
{{org.osgi.service.cm.Configuration#update(java.util.Dictionary<java.lang.String,
?>)}} and when (and there's high probability of this in 2.15.2) BP container
isn't yet started, it'll have those properties already available [when
{{<cm:property-placeholder>}} is
initialized|https://github.com/apache/aries/blob/35c92e2c941f82db9c8815467e9173a5d710ab08/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedProperties.java#L140-L144]:
{code:java}
public void init() throws Exception {
...
synchronized (lock) {
managedObjectManager.register(this, props);
Configuration config = CmUtils.getConfiguration(configAdmin,
persistentId);
if (config != null) {
properties = config.getProperties();
}
updated(properties);
}
}
{code}
Plus remember - there are two kinds of property placeholders: from Camel and
from Aries-Blueprint. Camel version delegates (in OSGi) to Aries version.
In order to have working {{trace="{{tracing}}"}} in 2.15.3+, you have to
override simple method:
{{org.apache.camel.test.junit4.CamelTestSupport#useOverridePropertiesWithPropertiesComponent}}.
Add:
{code:java}
@Override
protected Properties useOverridePropertiesWithPropertiesComponent() {
Properties props = new Properties();
props.setProperty("tracing", "true");
return props;
}
{code}
to
{{org.apache.camel.test.blueprint.ConfigAdminNoReloadLoadConfigurationFileTest}}
and {{trace="{{tracing}}"}} to
{{org/apache/camel/test/blueprint/configadmin-no-reload-loadfile.xml}} and
everything will work, because PropertyComponent will have needed property when
CamelContext is created.
In other words - my change in 2.15.3 related to predictability of
camel-test-blueprint tests can be summarized like this:
* we don't test (because of explicit synchronization) the scenarios, when
configadmin configurations are updated before blueprint container starts
* we test scenarios, where configadmin configuration updates should (when
there's {{<cm:property-placeholder ... update-strategy="reload">}}) lead to
reinitialization of blueprint container - and to make it predictable, we have
to synchrozize BP and configadmin events
It worked in 2.15.2 as a kind of side effect - just like the configadmin
configuration already existed when blueprint container was created for the
first time. but it's not different then specifying it explicitly with:
{code:xml}
<cm:property-placeholder persistent-id="stuff" update-strategy="none">
<cm:default-properties>
<cm:property name="x" value="y"/>
</cm:default-properties>
</cm:property-placeholder>
{code}
And yet another words: {{CamelBlueprintTestSupport}} calls
{{org.osgi.service.cm.Configuration#update(java.util.Dictionary<java.lang.String,?>)}}
*only* to change configadmin configs, not to init them.
I hope this resolves your issue.
> CamelBlueprintTestSupport - timing problems with property placeholder
> ---------------------------------------------------------------------
>
> Key: CAMEL-9313
> URL: https://issues.apache.org/jira/browse/CAMEL-9313
> Project: Camel
> Issue Type: Bug
> Components: camel-blueprint
> Affects Versions: 2.15.4
> Reporter: Andreas Siepert
> Assignee: Grzegorz Grzybek
> Fix For: 2.16.2, 2.15.5, 2.17.0
>
>
> The bugfix CAMEL-8948 seems to make older timing problems related to
> property-placeholders visible.
> To reproduce the problem i changed the test
> org.apache.camel.test.blueprint.ConfigAdminNoReloadLoadConfigurationFileTest
> from the component camel-test-blueprint a bit, respectively the context
> "org/apache/camel/test/blueprint/configadmin-no-reload-loadfile.xml":
> I added the trace Attribute to the camelContext:
> {code:xml}
> <camelContext xmlns="http://camel.apache.org/schema/blueprint"
> trace="{{tracing}}">
> {code}
> and added also the property to the etc/stuff.cfg
> {code}
> tracing=true
> {code}
> Until 2.15.2 this worked fine. From 2.15.3 on the property cannot be
> replaced any more.
> But, if setting a breakpoint in
> {{CamelBlueprintTestSupport#createBundleContext}} at
> loadConfigAdminConfigurationFile() (Line 123 in 2.15.4) - the error occurs
> even in older versions like 2.14 - so the timing problem seems to be there
> for a while but did not occur because the loading of the configAdminFile
> seems to be faster than the event handling during service registration
> triggered by the code some lines above.
> The issue can also be reproduced when replacing a property's String type
> with int in the MyCoolBean class and setting its value by using the
> placeholder like before but with an int value of course. The test run shows
> that the placeholder "${..\}" will not be replaced and leads to a
> NumberfFormatException.
> The production code that is under test works fine in karaf.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)