[ 
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)

Reply via email to