You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Petr Shypila <ik...@gmail.com> on 2015/06/30 11:02:55 UTC
Contentloader IT. Asynchronous bundle uninstall.
Hello Bertrand,
In regards to your comments in SLING-4541 I think you are right. I didn't
thought about that. And yes, I think the best way to solve this problem is
to subscribe for BundleEvent.UNINSTALLED event of these bundles. I'm going
to fix it today.
-Petr
Re: Contentloader IT. Asynchronous bundle uninstall.
Posted by Petr Shypila <ik...@gmail.com>.
Hello everyone,
I didn't create a patch for IT of contentloader module yet because it still
doesn't approved from your side. Is this approach works for you?
If yes, I would be happy to finish it, otherwise I would be happy to make
all required changes.
-Petr
2015-07-03 12:50 GMT+03:00 Petr Shypila <ik...@gmail.com>:
> Hello Bertrand,
>
> I have implemented basic implementation of retry-loop mechanism for IT of
> BundleContentLoader[1]. It mostly a copy of existing RetryLoop and should
> be refactored, but it shows the main idea of my approach. I have added
> static boolean to BundleContentLoader[2] indicates when BundleContentLoader
> is in idle state. As far as I can understand BundleContentLoader exists in
> single instance and every bundle uninstallation doesn't create new thread
> for these needs. Otherwise this approach will not work and I have no idea
> at this time how we can test it without major code changes.
>
> I have also created a factory class for retry-loop conditions to avoid
> code duplication, but the only thing I care about is required for factory
> session object[3] which I set before test starts.
>
> [1]
> https://github.com/PetrShypila/sling-builder/blob/9b807b82f74e00c389613714c625f9997398d7ac/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/it/RetryLoop.java
>
> [2]
> https://github.com/PetrShypila/sling-builder/blob/9b807b82f74e00c389613714c625f9997398d7ac/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/BundleContentLoader.java#L57
>
> [3]
> https://github.com/PetrShypila/sling-builder/blob/9b807b82f74e00c389613714c625f9997398d7ac/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/it/ConditionFactory.java#L53
>
> -Petr
>
>
> 2015-07-02 12:28 GMT+02:00 Bertrand Delacretaz <bd...@apache.org>:
>
>> Hi Petr,
>>
>> On Wed, Jul 1, 2015 at 9:40 PM, Petr Shypila <ik...@gmail.com> wrote:
>> > ...Speaking about a way how to detect when
>> > contentloader is idle. As I understand it is possible that after call:
>> > uninstallBundle(String symbolicName);
>> > and at the moment of checking that item exists(doesn't exists)
>> > contentloader could even not start to remove content, so it will be in
>> idle
>> > state. Make it probably sense to have some kind of flags which will
>> show if
>> > content was already removed?...
>>
>> Sorry but I don't understand what you mean by that, best is probably
>> for you to write a minimal prototype of how you think you can tackle
>> that issue. As I said minor changes to the src/main code might be ok
>> if they make testing easier.
>>
>> -Bertrand
>>
>
>
Re: Contentloader IT. Asynchronous bundle uninstall.
Posted by Petr Shypila <ik...@gmail.com>.
Hello Bertrand,
I have implemented basic implementation of retry-loop mechanism for IT of
BundleContentLoader[1]. It mostly a copy of existing RetryLoop and should
be refactored, but it shows the main idea of my approach. I have added
static boolean to BundleContentLoader[2] indicates when BundleContentLoader
is in idle state. As far as I can understand BundleContentLoader exists in
single instance and every bundle uninstallation doesn't create new thread
for these needs. Otherwise this approach will not work and I have no idea
at this time how we can test it without major code changes.
I have also created a factory class for retry-loop conditions to avoid code
duplication, but the only thing I care about is required for factory
session object[3] which I set before test starts.
[1]
https://github.com/PetrShypila/sling-builder/blob/9b807b82f74e00c389613714c625f9997398d7ac/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/it/RetryLoop.java
[2]
https://github.com/PetrShypila/sling-builder/blob/9b807b82f74e00c389613714c625f9997398d7ac/bundles/jcr/contentloader/src/main/java/org/apache/sling/jcr/contentloader/internal/BundleContentLoader.java#L57
[3]
https://github.com/PetrShypila/sling-builder/blob/9b807b82f74e00c389613714c625f9997398d7ac/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/it/ConditionFactory.java#L53
-Petr
2015-07-02 12:28 GMT+02:00 Bertrand Delacretaz <bd...@apache.org>:
> Hi Petr,
>
> On Wed, Jul 1, 2015 at 9:40 PM, Petr Shypila <ik...@gmail.com> wrote:
> > ...Speaking about a way how to detect when
> > contentloader is idle. As I understand it is possible that after call:
> > uninstallBundle(String symbolicName);
> > and at the moment of checking that item exists(doesn't exists)
> > contentloader could even not start to remove content, so it will be in
> idle
> > state. Make it probably sense to have some kind of flags which will show
> if
> > content was already removed?...
>
> Sorry but I don't understand what you mean by that, best is probably
> for you to write a minimal prototype of how you think you can tackle
> that issue. As I said minor changes to the src/main code might be ok
> if they make testing easier.
>
> -Bertrand
>
Re: Contentloader IT. Asynchronous bundle uninstall.
Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi Petr,
On Wed, Jul 1, 2015 at 9:40 PM, Petr Shypila <ik...@gmail.com> wrote:
> ...Speaking about a way how to detect when
> contentloader is idle. As I understand it is possible that after call:
> uninstallBundle(String symbolicName);
> and at the moment of checking that item exists(doesn't exists)
> contentloader could even not start to remove content, so it will be in idle
> state. Make it probably sense to have some kind of flags which will show if
> content was already removed?...
Sorry but I don't understand what you mean by that, best is probably
for you to write a minimal prototype of how you think you can tackle
that issue. As I said minor changes to the src/main code might be ok
if they make testing easier.
-Bertrand
Re: Contentloader IT. Asynchronous bundle uninstall.
Posted by Petr Shypila <ik...@gmail.com>.
Hi Bertrand,
Thank you for this advice. Speaking about a way how to detect when
contentloader is idle. As I understand it is possible that after call:
uninstallBundle(String symbolicName);
and at the moment of checking that item exists(doesn't exists)
contentloader could even not start to remove content, so it will be in idle
state. Make it probably sense to have some kind of flags which will show if
content was already removed?
-Petr
2015-07-01 8:02 GMT+02:00 Bertrand Delacretaz <bd...@apache.org>:
> Hi Petr,
>
> On Wed, Jul 1, 2015 at 12:52 AM, Petr Shypila <ik...@gmail.com> wrote:
> > ...Since now I need a RetryLoop I need a new dependency as well. And it
> should
> > be loaded to Karaf...
>
> We're not using Karaf in those tests, it's pax exam that's starting
> them - but yes if you use the RetryLoop you need to load the
> org.apache.sling.testing.tools bundle.
>
> In your case the bundle is probably not starting due to missing
> dependencies, you would see that from messages like "Error starting
> mvn:org.apache.sling/org.apache.sling.testing.tools/1.0.10" in the
> test log.
>
> But considering that that bundle has several dependencies that are
> unrelated to what we are testing here it's probably better to
> implement your own simple retry mechanism for these tests.
>
> Note that the way you tried to use RetryLoop wouldn't work anyway,
> that class waits for its condition to *become* true, so if you wait
> like this for example
>
> public boolean isTrue() throws Exception {
> // Node should not be deleted after bundle was uninstalled
> return session.itemExists(testNodeName + ".json/jcr:content");
> }
>
> If the node is deleted but only after the condition executes the first
> time, your test won't detect that - so you'll need a better mechanism
> to test such things. You might need to detect when the contentloader
> becomes idle after detecting bundle changes, if changes are needed to
> the module code to make that testing easier we can discuss that.
>
> -Bertrand
>
Re: Contentloader IT. Asynchronous bundle uninstall.
Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi Petr,
On Wed, Jul 1, 2015 at 12:52 AM, Petr Shypila <ik...@gmail.com> wrote:
> ...Since now I need a RetryLoop I need a new dependency as well. And it should
> be loaded to Karaf...
We're not using Karaf in those tests, it's pax exam that's starting
them - but yes if you use the RetryLoop you need to load the
org.apache.sling.testing.tools bundle.
In your case the bundle is probably not starting due to missing
dependencies, you would see that from messages like "Error starting
mvn:org.apache.sling/org.apache.sling.testing.tools/1.0.10" in the
test log.
But considering that that bundle has several dependencies that are
unrelated to what we are testing here it's probably better to
implement your own simple retry mechanism for these tests.
Note that the way you tried to use RetryLoop wouldn't work anyway,
that class waits for its condition to *become* true, so if you wait
like this for example
public boolean isTrue() throws Exception {
// Node should not be deleted after bundle was uninstalled
return session.itemExists(testNodeName + ".json/jcr:content");
}
If the node is deleted but only after the condition executes the first
time, your test won't detect that - so you'll need a better mechanism
to test such things. You might need to detect when the contentloader
becomes idle after detecting bundle changes, if changes are needed to
the module code to make that testing easier we can discuss that.
-Bertrand
Re: Contentloader IT. Asynchronous bundle uninstall.
Posted by Petr Shypila <ik...@gmail.com>.
Hi Bertrand,
Thank you very much again for all your help. Unfortunately I still have
some problems.
Since now I need a RetryLoop I need a new dependency as well. And it should
be loaded to Karaf. As I understand RetryLoop's module should be loaded to
Karaf as a wrapped bundle[1]. That's what I'm trying to do[2] and [3].
Module's dependency is also declared in my pom file. So what am I doing
wrong?
[1] https://ops4j1.jira.com/wiki/display/paxurl/Wrap+Protocol
[2]
https://github.com/PetrShypila/sling-builder/blob/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/it/BundleContentLoaderIT.java
[3]
https://github.com/PetrShypila/sling-builder/blob/trunk/bundles/jcr/contentloader/src/test/java/org/apache/sling/jcr/contentloader/it/P.java
-Petr
2015-06-30 13:07 GMT+02:00 Bertrand Delacretaz <bd...@apache.org>:
> Hi Petr,
>
> On Tue, Jun 30, 2015 at 12:24 PM, Petr Shypila <ik...@gmail.com>
> wrote:
> > ...I will add Thread.sleep(300)(?) calls before checking that content was
> > uninstalled..
>
> That's ugly ;-)
>
> If you need a retry loop, you can either reuse the one from [1] (which
> is a bit verbose to use) or implement something similar - wait a small
> amount of time between checks, up to a timeout.
>
> > ...By the way, first time I tried to understand how I can extend
> > your ContentBundleTestBase class and use with several bundles, but after
> I
> > didn't find a nice solution for that, I did it in my way...
>
> Ok that works as well.
> -Bertrand
>
> [1]
> https://svn.apache.org/repos/asf/sling/trunk/testing/tools/src/main/java/org/apache/sling/testing/tools/retry/RetryLoop.java
>
Re: Contentloader IT. Asynchronous bundle uninstall.
Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi Petr,
On Tue, Jun 30, 2015 at 12:24 PM, Petr Shypila <ik...@gmail.com> wrote:
> ...I will add Thread.sleep(300)(?) calls before checking that content was
> uninstalled..
That's ugly ;-)
If you need a retry loop, you can either reuse the one from [1] (which
is a bit verbose to use) or implement something similar - wait a small
amount of time between checks, up to a timeout.
> ...By the way, first time I tried to understand how I can extend
> your ContentBundleTestBase class and use with several bundles, but after I
> didn't find a nice solution for that, I did it in my way...
Ok that works as well.
-Bertrand
[1] https://svn.apache.org/repos/asf/sling/trunk/testing/tools/src/main/java/org/apache/sling/testing/tools/retry/RetryLoop.java
Re: Contentloader IT. Asynchronous bundle uninstall.
Posted by Petr Shypila <ik...@gmail.com>.
Hi Bertrand,
Yes, I think it's even simpler to implement, because I didn't work with
OSGi events before. I just thought It's not a good way to tests that. Ok,
then I will add Thread.sleep(300)(?) calls before checking that content was
uninstalled.
By the way, first time I tried to understand how I can extend
your ContentBundleTestBase class and use with several bundles, but after I
didn't find a nice solution for that, I did it in my way.
-Petr
2015-06-30 12:03 GMT+02:00 Bertrand Delacretaz <bd...@apache.org>:
> Hi Petr,
>
> On Tue, Jun 30, 2015 at 11:02 AM, Petr Shypila <ik...@gmail.com>
> wrote:
> > ...I think the best way to solve this problem is
> > to subscribe for BundleEvent.UNINSTALLED event of these bundles....
>
> That might not be sufficient, as I suppose the content loader acts
> asynchronously, potentially after that event is emitted.
>
> To verify that you might add some sleep(...) calls in the content
> loader, and see if that causes your tests to fail.
>
> Another strategy would be just to add some retry loops in your tests,
> to wait for what you expect to happen (a property being overwritten
> etc.), with a timeout - that might solve most of those cases.
>
> -Bertrand
>
Re: Contentloader IT. Asynchronous bundle uninstall.
Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi Petr,
On Tue, Jun 30, 2015 at 11:02 AM, Petr Shypila <ik...@gmail.com> wrote:
> ...I think the best way to solve this problem is
> to subscribe for BundleEvent.UNINSTALLED event of these bundles....
That might not be sufficient, as I suppose the content loader acts
asynchronously, potentially after that event is emitted.
To verify that you might add some sleep(...) calls in the content
loader, and see if that causes your tests to fail.
Another strategy would be just to add some retry loops in your tests,
to wait for what you expect to happen (a property being overwritten
etc.), with a timeout - that might solve most of those cases.
-Bertrand