Adobe-Consulting-Services / acs-aem-commons

http://adobe-consulting-services.github.io/acs-aem-commons/
Apache License 2.0
456 stars 604 forks source link

[Blocks v4.0.0] Default values from OSGI annotations (OCD) are not injected in activate methods. #1652

Closed niekraaijmakers closed 5 years ago

niekraaijmakers commented 5 years ago

Sorry for the brief issue instead of detailed. But it's christmas and I don't want to spend too much time, and I just want to flag this. I can provide more details later.

Required Information

AEM 6.3 / 6.4

Expected Behavior

On the activate method of services, with a ObjectClassDefinition object, using the new annotations, the object returns default values as specified by the @ObjectClassDefinition / @AttributeDefinition.

Actual Behavior

The default values are not returned, only values present are returned.

Steps to Reproduce

Examples: (start AEM in debug mode and attach IDE debugger) com.adobe.acs.commons.fam.impl.ThrottledTaskRunnerImpl > activate line 316

Specified values in OSGI configs are being picked up, but the default values from the config are ignored.

I also faced this with the HTTP cache module where I tried to leverage this mechanism instead of the old Map<String,Object properties.

davidjgonzalez commented 5 years ago

Thanks @Sc0rpic0m and Merry Christmas!

joerghoh commented 5 years ago

For this specific example:

    @AttributeDefinition(name = "Max cpu %", description = "Range is 0..1; -1 means disable this check", defaultValue = "0.75")
        int max_cpu();

I don't think that '0.75' can be converted safely to an int ...

niekraaijmakers commented 5 years ago

https://issues.apache.org/jira/browse/FELIX-5404 https://github.com/apache/felix/commit/f0e78ad562f00fcd009a9d43678962897e29d8a0#diff-5a8d484bf513adc7e77f7a81b8743b1c

niekraaijmakers commented 5 years ago

Seems to me, that on felix scr-2.1.0, this fix is not present. Not sure from which version above mentioned fix is introduced. Let me check on AEM 6.5 which version is present and if the problem is still there.

niekraaijmakers commented 5 years ago

I checked on 6.4, SP 2 and 3, both versions of the felix src plugin are still 2.1.0 :(

niekraaijmakers commented 5 years ago

Ok on AEM 6.5, we are using the latest and greatest, so then it works fine. Is 4.0.0 release only meant for AEM 6.5? Because then we can close this bug I guess. Otherwise, we will require another AEM SP or something for 6.4.

Weird though. Isn't this working for other people on earlier AEM versions? I must be missing something? I would like to know if anybody has the annotation based approach working for any project on any version earlier then 6.5, and which felix scr version they have.

davidjgonzalez commented 5 years ago

I think this is the ~same issue as https://github.com/Adobe-Consulting-Services/acs-aem-commons/issues/1645 where WF process labels are not coming through from default values.

This is odd since in ASC, new OSGi w/ OCD and defaults are used just fine: For (a simple) example the Labels "work" all the Computed Properties [1] in ASC and those are set via default values.

[1] https://github.com/Adobe-Marketing-Cloud/asset-share-commons/blob/master/core/src/main/java/com/adobe/aem/commons/assetshare/content/properties/impl/ExpiredImpl.java

joerghoh commented 5 years ago

@Sc0rpic0m do you have more examples where this is happening? I haven't observed any problems with this approach. Just make sure that the value can converted properly; in the example 0.75 cannot be converted to an int, which is quite obvious...

niekraaijmakers commented 5 years ago

Well. I pulled in your code and tested it on my machine, but still the same issue (on the throttled task runner). But it works for your machine. Which AEM + SP do you use? I tested this with fresh AEM instances as well (6.3 / 6.4 SP 2, 6.4 SP 3).

And just curious, which felix scr bundle is running on your aem, higher by 2.1.0 by any chance?

bildschirmfoto 2019-01-03 um 22 44 14

I see that the activate method (call stack) still comes from the package org.apache.felix.scr.. is this correct as impl bundle? I just need 1 instance where it works and then I can work from there I guess. Any activate method using the @Designated config object instead of the classic Map properties where the default values work. I cannot find one.

niekraaijmakers commented 5 years ago

What I can find on the throttled task runner that works is the default on the interface method instead of defaultValue in the annotation.

@AttributeDefinition(name = "Max threads", description = "Default is 4, recommended not to exceed the number of CPU cores",defaultValue = "4") int max_threads() default 4;

And: https://github.com/Adobe-Marketing-Cloud/asset-share-commons/blob/5fdc057951d5f2d3f16d0765abb4f9466054431f/core/src/main/java/com/adobe/aem/commons/assetshare/content/properties/impl/ExpiredImpl.java

That seems to make it work. Is this the way forward? I actually tested on AEM 6.5 and the same problem comes forward even with the new felix scr bundle!