Adobe-Consulting-Services / acs-aem-commons

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

I18NInjector is called on Object field in SlingModel #2158

Closed beo4 closed 4 years ago

beo4 commented 4 years ago

Required Information

Expected Behavior

A String or String Array should be handed over to Object Filed in a SlingModel

Actual Behavior

After handing over a htl Parameter to an Object in class I18N Object gets injected

Steps to Reproduce

See https://github.com/eggsunimediaGmbH/acs-commons/blob/master/core/src/test/java/de/eggs/core/models/HelloWorldModelTest.java

Links

https://github.com/eggsunimediaGmbH/acs-commons/blob/master/core/src/test/java/de/eggs/core/models/HelloWorldModelTest.java

joerghoh commented 4 years ago

For some reasons the 18NInjector handles the adaptions to Object, which is unexpected if you come from older versions of ACS AEM Commons, where this Injector isn't part of.

One option would be to change the service ranking, another to disable all custom injectors by default (and requiring an available configuration, as we do with other services as well).

On the other hand one could argue, that injecting an Object is very uncommon per se, and shouldn't be done at all; and if you have to use it, you have to use @Source and define which injector you want to use. In that case the current code is ok, but we should extend the documentation for this specific case.

@davidjgonzalez @badvision WDYT?

joerghoh commented 4 years ago

I would tend to the suggestion 2 (leave it as is and improve the documentation); injecting Object references might not only lead to issues with the I18Injector, but also with others; thus annotating it with the type of the injector you expect to provide the injection is probably the best way to go forward.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.