Re: ResourceResolverFactory restart behaviour

[Konrad Windszus]

Yes, SLING-4360 added a check if the RRF is live to the method 
ResourceResolver.isLive(). The problem is that you have to call that method 
explicitly.
For me the use case is rather to detect programming mistakes like not listening 
for ResourceResolverFactory restarts. 
What about implicitly calling RR.isLive for every other method?
Actually the Javadoc for all RR methods already states it will throw an 
IllegalArgumentException if the RR has been closed already. I would expect the 
same exception in case the resource resolver factory has been closed (maybe 
with a different, more explicit error message).
What was the reason why only the RR.isLive method was modified with SLING-4360?
Konrad


> On 14 Sep 2015, at 20:57, Carsten Ziegeler  wrote:
> 
> Am 14.09.15 um 18:04 schrieb Konrad Windszus:
>> Thanks Carsten, indeed I am hitting SLING-4974.
>> Could you point me to the commit which closes the resource resolvers after 
>> the restart of the factory (or after some required providers become 
>> unavailable)?
>> I couldn’t find any related commit in 
>> https://github.com/apache/sling/commits/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl
>>  
>> .
>> In which version of the resource resolver bundle is that implemented?
>> At least with resource resolver 1.2.4 this is not the case.
> 
> Then we might have a bug still in there, it's SLING-4360 which is
> version 1.1.12. Afaik, we did some tests back then and it seemed to
> work, but maybe it's not handling all cases properly?
> 
> Regards
> Carsten
> 
>> Thanks,
>> Konrad
>> 
>>> On 14 Sep 2015, at 17:55, Carsten Ziegeler  wrote:
>>> 
>>> Am 14.09.15 um 17:51 schrieb Konrad Windszus:
 Currently if the resource resolver factory is restarted (e.g. because the 
 configuration has been changed) all resource resolvers which were being 
 requested beforehand become almost useless, because they have no longer 
 any providers being bound! This means those resolvers can only be used to 
 retrieve the entry points below “/“ but nothing beyond that.
 Therefore if you forget to refresh a resource resolver after the resource 
 resolver factory has been restarted you get very weird results. One 
 example of this behaviour is being described in 
 https://issues.apache.org/jira/browse/SLING-5023.
 What about marking those resource resolvers as invalid (because they do no 
 longer have the required resource providers being bound) and let all 
 methods being called on those resource resolvers fail with an 
 IllegalStateException?
 That way those kind of programming mistakes would be more obvious!
 Thanks for your opinions on that.
 Konrad
 
 
>>> Hi
>>> 
>>> I think you're hitting a dub of SLING-4974 and afaik with the latest
>>> resource resolver impl we mark the resolvers as closed :)
>>> 
>>> Carsten
>>> 
>>> -- 
>>> Carsten Ziegeler
>>> Adobe Research Switzerland
>>> cziege...@apache.org
>> 
>> 
> 
> 
> -- 
> Carsten Ziegeler
> Adobe Research Switzerland
> cziege...@apache.org

Re: ResourceResolverFactory restart behaviour

[Konrad Windszus]

Ok, thanks for the confirmation. I created issue 
https://issues.apache.org/jira/browse/SLING-5025 for that.
The only question is, whether the check for context.isLive() should also be 
done within the checkClosed()?

In theory I guess one could still use a resource resolver if one of the used 
providers is gone. Of course this is not the case for the JcrResourceProvider 
but since this is a required provider it would lead to an unregistration of the 
whole RRF.

> On 15 Sep 2015, at 09:35, Carsten Ziegeler  wrote:
> 
> Am 15.09.15 um 08:37 schrieb Konrad Windszus:
>> Yes, SLING-4360 added a check if the RRF is live to the method 
>> ResourceResolver.isLive(). The problem is that you have to call that method 
>> explicitly.
>> For me the use case is rather to detect programming mistakes like not 
>> listening for ResourceResolverFactory restarts. 
>> What about implicitly calling RR.isLive for every other method?
> 
> Oh, I see, every method calls checkClosed() which doesn't check the
> active flag. I think we should add this there as well.
> 
>> Actually the Javadoc for all RR methods already states it will throw an 
>> IllegalArgumentException if the RR has been closed already. I would expect 
>> the same exception in case the resource resolver factory has been closed 
>> (maybe with a different, more explicit error message).
>> What was the reason why only the RR.isLive method was modified with 
>> SLING-4360?
> 
> Good question :) I guess, it's just an oversight (aka bug) :)
> 
> Carsten
> 
>> Konrad
>> 
>> 
>>> On 14 Sep 2015, at 20:57, Carsten Ziegeler  wrote:
>>> 
>>> Am 14.09.15 um 18:04 schrieb Konrad Windszus:
 Thanks Carsten, indeed I am hitting SLING-4974.
 Could you point me to the commit which closes the resource resolvers after 
 the restart of the factory (or after some required providers become 
 unavailable)?
 I couldn’t find any related commit in 
 https://github.com/apache/sling/commits/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl
  
 .
 In which version of the resource resolver bundle is that implemented?
 At least with resource resolver 1.2.4 this is not the case.
>>> 
>>> Then we might have a bug still in there, it's SLING-4360 which is
>>> version 1.1.12. Afaik, we did some tests back then and it seemed to
>>> work, but maybe it's not handling all cases properly?
>>> 
>>> Regards
>>> Carsten
>>> 
 Thanks,
 Konrad
 
> On 14 Sep 2015, at 17:55, Carsten Ziegeler  wrote:
> 
> Am 14.09.15 um 17:51 schrieb Konrad Windszus:
>> Currently if the resource resolver factory is restarted (e.g. because 
>> the configuration has been changed) all resource resolvers which were 
>> being requested beforehand become almost useless, because they have no 
>> longer any providers being bound! This means those resolvers can only be 
>> used to retrieve the entry points below “/“ but nothing beyond that.
>> Therefore if you forget to refresh a resource resolver after the 
>> resource resolver factory has been restarted you get very weird results. 
>> One example of this behaviour is being described in 
>> https://issues.apache.org/jira/browse/SLING-5023.
>> What about marking those resource resolvers as invalid (because they do 
>> no longer have the required resource providers being bound) and let all 
>> methods being called on those resource resolvers fail with an 
>> IllegalStateException?
>> That way those kind of programming mistakes would be more obvious!
>> Thanks for your opinions on that.
>> Konrad
>> 
>> 
> Hi
> 
> I think you're hitting a dub of SLING-4974 and afaik with the latest
> resource resolver impl we mark the resolvers as closed :)
> 
> Carsten
> 
> -- 
> Carsten Ziegeler
> Adobe Research Switzerland
> cziege...@apache.org
 
 
>>> 
>>> 
>>> -- 
>>> Carsten Ziegeler
>>> Adobe Research Switzerland
>>> cziege...@apache.org
>> 
>> 
> 
> 
> -- 
> Carsten Ziegeler
> Adobe Research Switzerland
> cziege...@apache.org

Re: ResourceResolverFactory restart behaviour

[Carsten Ziegeler]

Am 15.09.15 um 08:37 schrieb Konrad Windszus:
> Yes, SLING-4360 added a check if the RRF is live to the method 
> ResourceResolver.isLive(). The problem is that you have to call that method 
> explicitly.
> For me the use case is rather to detect programming mistakes like not 
> listening for ResourceResolverFactory restarts. 
> What about implicitly calling RR.isLive for every other method?

Oh, I see, every method calls checkClosed() which doesn't check the
active flag. I think we should add this there as well.

> Actually the Javadoc for all RR methods already states it will throw an 
> IllegalArgumentException if the RR has been closed already. I would expect 
> the same exception in case the resource resolver factory has been closed 
> (maybe with a different, more explicit error message).
> What was the reason why only the RR.isLive method was modified with 
> SLING-4360?

Good question :) I guess, it's just an oversight (aka bug) :)

Carsten

> Konrad
> 
> 
>> On 14 Sep 2015, at 20:57, Carsten Ziegeler  wrote:
>>
>> Am 14.09.15 um 18:04 schrieb Konrad Windszus:
>>> Thanks Carsten, indeed I am hitting SLING-4974.
>>> Could you point me to the commit which closes the resource resolvers after 
>>> the restart of the factory (or after some required providers become 
>>> unavailable)?
>>> I couldn’t find any related commit in 
>>> https://github.com/apache/sling/commits/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl
>>>  
>>> .
>>> In which version of the resource resolver bundle is that implemented?
>>> At least with resource resolver 1.2.4 this is not the case.
>>
>> Then we might have a bug still in there, it's SLING-4360 which is
>> version 1.1.12. Afaik, we did some tests back then and it seemed to
>> work, but maybe it's not handling all cases properly?
>>
>> Regards
>> Carsten
>>
>>> Thanks,
>>> Konrad
>>>
 On 14 Sep 2015, at 17:55, Carsten Ziegeler  wrote:

 Am 14.09.15 um 17:51 schrieb Konrad Windszus:
> Currently if the resource resolver factory is restarted (e.g. because the 
> configuration has been changed) all resource resolvers which were being 
> requested beforehand become almost useless, because they have no longer 
> any providers being bound! This means those resolvers can only be used to 
> retrieve the entry points below “/“ but nothing beyond that.
> Therefore if you forget to refresh a resource resolver after the resource 
> resolver factory has been restarted you get very weird results. One 
> example of this behaviour is being described in 
> https://issues.apache.org/jira/browse/SLING-5023.
> What about marking those resource resolvers as invalid (because they do 
> no longer have the required resource providers being bound) and let all 
> methods being called on those resource resolvers fail with an 
> IllegalStateException?
> That way those kind of programming mistakes would be more obvious!
> Thanks for your opinions on that.
> Konrad
>
>
 Hi

 I think you're hitting a dub of SLING-4974 and afaik with the latest
 resource resolver impl we mark the resolvers as closed :)

 Carsten

 -- 
 Carsten Ziegeler
 Adobe Research Switzerland
 cziege...@apache.org
>>>
>>>
>>
>>
>> -- 
>> Carsten Ziegeler
>> Adobe Research Switzerland
>> cziege...@apache.org
> 
> 


-- 
Carsten Ziegeler
Adobe Research Switzerland
cziege...@apache.org

Re: ResourceResolverFactory restart behaviour

[Carsten Ziegeler]

Am 15.09.15 um 09:50 schrieb Konrad Windszus:
> Ok, thanks for the confirmation. I created issue 
> https://issues.apache.org/jira/browse/SLING-5025 for that.
> The only question is, whether the check for context.isLive() should also be 
> done within the checkClosed()?
> 
Yes, I think checkClosed should check the same stuff as RR.isLive is
currently doing.

Carsten


-- 
Carsten Ziegeler
Adobe Research Switzerland
cziege...@apache.org

ResourceResolverFactory restart behaviour

[Konrad Windszus]

Currently if the resource resolver factory is restarted (e.g. because the 
configuration has been changed) all resource resolvers which were being 
requested beforehand become almost useless, because they have no longer any 
providers being bound! This means those resolvers can only be used to retrieve 
the entry points below “/“ but nothing beyond that.
Therefore if you forget to refresh a resource resolver after the resource 
resolver factory has been restarted you get very weird results. One example of 
this behaviour is being described in 
https://issues.apache.org/jira/browse/SLING-5023.
What about marking those resource resolvers as invalid (because they do no 
longer have the required resource providers being bound) and let all methods 
being called on those resource resolvers fail with an IllegalStateException?
That way those kind of programming mistakes would be more obvious!
Thanks for your opinions on that.
Konrad

Re: ResourceResolverFactory restart behaviour

[Carsten Ziegeler]

Am 14.09.15 um 17:51 schrieb Konrad Windszus:
> Currently if the resource resolver factory is restarted (e.g. because the 
> configuration has been changed) all resource resolvers which were being 
> requested beforehand become almost useless, because they have no longer any 
> providers being bound! This means those resolvers can only be used to 
> retrieve the entry points below “/“ but nothing beyond that.
> Therefore if you forget to refresh a resource resolver after the resource 
> resolver factory has been restarted you get very weird results. One example 
> of this behaviour is being described in 
> https://issues.apache.org/jira/browse/SLING-5023.
> What about marking those resource resolvers as invalid (because they do no 
> longer have the required resource providers being bound) and let all methods 
> being called on those resource resolvers fail with an IllegalStateException?
> That way those kind of programming mistakes would be more obvious!
> Thanks for your opinions on that.
> Konrad
> 
> 
Hi

I think you're hitting a dub of SLING-4974 and afaik with the latest
resource resolver impl we mark the resolvers as closed :)

Carsten

-- 
Carsten Ziegeler
Adobe Research Switzerland
cziege...@apache.org

Re: ResourceResolverFactory restart behaviour

[Konrad Windszus]

Thanks Carsten, indeed I am hitting SLING-4974.
Could you point me to the commit which closes the resource resolvers after the 
restart of the factory (or after some required providers become unavailable)?
I couldn’t find any related commit in 
https://github.com/apache/sling/commits/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl
 
.
In which version of the resource resolver bundle is that implemented?
At least with resource resolver 1.2.4 this is not the case.
Thanks,
Konrad

> On 14 Sep 2015, at 17:55, Carsten Ziegeler  wrote:
> 
> Am 14.09.15 um 17:51 schrieb Konrad Windszus:
>> Currently if the resource resolver factory is restarted (e.g. because the 
>> configuration has been changed) all resource resolvers which were being 
>> requested beforehand become almost useless, because they have no longer any 
>> providers being bound! This means those resolvers can only be used to 
>> retrieve the entry points below “/“ but nothing beyond that.
>> Therefore if you forget to refresh a resource resolver after the resource 
>> resolver factory has been restarted you get very weird results. One example 
>> of this behaviour is being described in 
>> https://issues.apache.org/jira/browse/SLING-5023.
>> What about marking those resource resolvers as invalid (because they do no 
>> longer have the required resource providers being bound) and let all methods 
>> being called on those resource resolvers fail with an IllegalStateException?
>> That way those kind of programming mistakes would be more obvious!
>> Thanks for your opinions on that.
>> Konrad
>> 
>> 
> Hi
> 
> I think you're hitting a dub of SLING-4974 and afaik with the latest
> resource resolver impl we mark the resolvers as closed :)
> 
> Carsten
> 
> -- 
> Carsten Ziegeler
> Adobe Research Switzerland
> cziege...@apache.org

Re: ResourceResolverFactory restart behaviour

[Carsten Ziegeler]

Am 14.09.15 um 18:04 schrieb Konrad Windszus:
> Thanks Carsten, indeed I am hitting SLING-4974.
> Could you point me to the commit which closes the resource resolvers after 
> the restart of the factory (or after some required providers become 
> unavailable)?
> I couldn’t find any related commit in 
> https://github.com/apache/sling/commits/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl
>  
> .
> In which version of the resource resolver bundle is that implemented?
> At least with resource resolver 1.2.4 this is not the case.

Then we might have a bug still in there, it's SLING-4360 which is
version 1.1.12. Afaik, we did some tests back then and it seemed to
work, but maybe it's not handling all cases properly?

Regards
Carsten

> Thanks,
> Konrad
> 
>> On 14 Sep 2015, at 17:55, Carsten Ziegeler  wrote:
>>
>> Am 14.09.15 um 17:51 schrieb Konrad Windszus:
>>> Currently if the resource resolver factory is restarted (e.g. because the 
>>> configuration has been changed) all resource resolvers which were being 
>>> requested beforehand become almost useless, because they have no longer any 
>>> providers being bound! This means those resolvers can only be used to 
>>> retrieve the entry points below “/“ but nothing beyond that.
>>> Therefore if you forget to refresh a resource resolver after the resource 
>>> resolver factory has been restarted you get very weird results. One example 
>>> of this behaviour is being described in 
>>> https://issues.apache.org/jira/browse/SLING-5023.
>>> What about marking those resource resolvers as invalid (because they do no 
>>> longer have the required resource providers being bound) and let all 
>>> methods being called on those resource resolvers fail with an 
>>> IllegalStateException?
>>> That way those kind of programming mistakes would be more obvious!
>>> Thanks for your opinions on that.
>>> Konrad
>>>
>>>
>> Hi
>>
>> I think you're hitting a dub of SLING-4974 and afaik with the latest
>> resource resolver impl we mark the resolvers as closed :)
>>
>> Carsten
>>
>> -- 
>> Carsten Ziegeler
>> Adobe Research Switzerland
>> cziege...@apache.org
> 
> 


-- 
Carsten Ziegeler
Adobe Research Switzerland
cziege...@apache.org