You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2020/08/31 16:37:21 UTC
[GitHub] [sling-org-apache-sling-scripting-sightly] paul-bjorkstrand opened a new pull request #8: Properly handle the adaptable argument for a Sling Model class
paul-bjorkstrand opened a new pull request #8:
URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly/pull/8
When Sling Models instantiation was added in SLING-9320 ([commit][1]), it did not treat the `adaptable` argument the same for all types of instantiations (Sling Models vs basic Sling Adaptable). This fixes that by rearranging the instantiation attempt order, which puts the argument provided adaptable through `ModelFactory#createModel` just like the request and resource would be.
This is a subtle semantic change, and likely warrants a minor version update.
[1]: https://github.com/apache/sling-org-apache-sling-scripting-sightly/commit/bcd46027e09182911c55bb244d42debb1bd367c7
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-scripting-sightly] paul-bjorkstrand commented on pull request #8: SLING-9715 - The JavaUseProvider does not properly handle the adaptable argument for Sling Model classes
Posted by GitBox <gi...@apache.org>.
paul-bjorkstrand commented on pull request #8:
URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly/pull/8#issuecomment-684897864
Ah, I gotta watch that copy-pasta. I sheepishly admit that my quick test with it was using the current resource, rather than a different one, so clearly I did not exercise that path. 😞
Thanks for that catch, @raducotescu.
I will also say, that I was inches away from creating a unit test for that, but shied away due to the sheer weight of the setup code needed. To make it more easily testable, will require some significant reorganization of that class' methods.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-scripting-sightly] raducotescu commented on pull request #8: Properly handle the adaptable argument for a Sling Model class
Posted by GitBox <gi...@apache.org>.
raducotescu commented on pull request #8:
URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly/pull/8#issuecomment-683900253
@paul-bjorkstrand, can you please create a SLING issue and link the PR to it? I'll have a look tomorrow, to make sure that the IT still passes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-scripting-sightly] raducotescu closed pull request #8: SLING-9715 - The JavaUseProvider does not properly handle the adaptable argument for Sling Model classes
Posted by GitBox <gi...@apache.org>.
raducotescu closed pull request #8:
URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly/pull/8
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-scripting-sightly] paul-bjorkstrand commented on pull request #8: Properly handle the adaptable argument for a Sling Model class
Posted by GitBox <gi...@apache.org>.
paul-bjorkstrand commented on pull request #8:
URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly/pull/8#issuecomment-684127336
@raducotescu: https://issues.apache.org/jira/browse/SLING-9715
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-scripting-sightly] raducotescu commented on pull request #8: SLING-9715 - The JavaUseProvider does not properly handle the adaptable argument for Sling Model classes
Posted by GitBox <gi...@apache.org>.
raducotescu commented on pull request #8:
URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly/pull/8#issuecomment-684832256
Applied a slightly modified patch in 960d9cd. Had to replace:
```java
if (adaptable != null && modelFactory.canCreateFromAdaptable(adaptable, cls)) {
LOG.debug("Trying to instantiate class {} as Sling Model from provided adaptable.", cls);
return ProviderOutcome.notNullOrFailure(modelFactory.createModel(request, cls));
}
```
with
```java
if (adaptable != null && modelFactory.canCreateFromAdaptable(adaptable, cls)) {
LOG.debug("Trying to instantiate class {} as Sling Model from provided adaptable.", cls);
return ProviderOutcome.notNullOrFailure(modelFactory.createModel(adaptable, cls));
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-scripting-sightly] raducotescu commented on pull request #8: SLING-9715 - The JavaUseProvider does not properly handle the adaptable argument for Sling Model classes
Posted by GitBox <gi...@apache.org>.
raducotescu commented on pull request #8:
URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly/pull/8#issuecomment-684899749
I think it shouldn't be that difficult to add some unit tests, using spies, but I've added an IT for this feature. See https://github.com/apache/sling-org-apache-sling-scripting-sightly-testing-content/commit/20f1108 and https://github.com/apache/sling-org-apache-sling-scripting-sightly-testing/commit/f266d23.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-scripting-sightly] raducotescu edited a comment on pull request #8: SLING-9715 - The JavaUseProvider does not properly handle the adaptable argument for Sling Model classes
Posted by GitBox <gi...@apache.org>.
raducotescu edited a comment on pull request #8:
URL: https://github.com/apache/sling-org-apache-sling-scripting-sightly/pull/8#issuecomment-684832256
Applied a slightly modified patch in 960d9cd. Had to replace:
```java
if (adaptable != null && modelFactory.canCreateFromAdaptable(adaptable, cls)) {
LOG.debug("Trying to instantiate class {} as Sling Model from provided adaptable.", cls);
return ProviderOutcome.notNullOrFailure(modelFactory.createModel(request, cls));
}
```
with
```java
if (adaptable != null && modelFactory.canCreateFromAdaptable(adaptable, cls)) {
LOG.debug("Trying to instantiate class {} as Sling Model from provided adaptable.", cls);
return ProviderOutcome.notNullOrFailure(modelFactory.createModel(adaptable, cls));
}
```
Thanks for the patch, @paul-bjorkstrand!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org