Add a method to detect which properties were changed in a reload #47

bbossola · 2013-07-28T23:26:10Z

The HotReload is super cool :) however I think that generally it would be nice to know which properties has changed so that you can re-configure only the affected services, It can be embedded into the ReloadEvent, or it could be possible have a helper method (like I have in my code now) that is able to do a "diff" in the properties for me.

lviggiano · 2013-07-29T11:23:22Z

👍

I thought about that, to have a listener that receives a PropertyChangeEvent containing { key, newValue, oldValue) (newValue and oldValue may be null to mean that a property has been added or removed)
Would that be ok?

cfg.addPropertyChangeListener(new PropertyChangeListener() {
    void propertyChanged(PropertyChangeEvent evt) {
         //....
    }
}

The above will get triggered for every property changed. (when reload happens it will be triggered as many times as the number of the properties loaded that have been changed)

Another option would be to register for a specific propertyKeyChange:

cfg.addPropertyChangeListener(new PropertyKeyChangeListener("foobar") {
    void propertyKeyChanged(PropertyChangeEvent evt) {
         //....
    }
}

The above will only be triggered when a specific property is changed.

I think I'll implement both. It should not be difficult.

Would this cover your use case?

lviggiano · 2013-07-29T20:15:09Z

On master branch, owner should be able to hot-reload properties also when they are loaded from the classpath. I need to test this more, and update the documentation (that states the opposite)

lviggiano · 2013-07-30T12:29:27Z

Also, it would be nice to allow the listener to rollback the property change (or the entire reload), for instance, throwing a PropertyChangeRollbackException, or ReloadRollbackException, what do you think?

This can work as a programmable 'validation' mechanism.

bbossola · 2013-07-30T17:06:20Z

Yes, it would be cool :) but you will also need to implement some sort of
two phase commit: first check all the listeners, when everybody is okay
then execute the change.

lviggiano · 2013-07-30T19:37:35Z

That's not the hard part, I think. And that was exactly what I had in mind.

But there are some cases to take in mind.

Suppose property X is changed, and the value passes from A to B.

There are two listeners registered for changes on property X.

1st listener gets notified, and for him X := B.
2nd listener gets notified and throws the exception to say "no, roll back X := A"
3rd listener doesn't get notified, since the 2nd cancelled the change.

So... we have this situation.
For the 1st listener X is equal to B. For the 2nd and 3rd listeners X is equal to A...

So... troubles :)

One could send another event to 1st listener to say that X gets assigned back to A... but... what if then the 1st listener throws an exception? .... it's a matter to save "capra e cavoli" ;)

bbossola · 2013-07-31T09:27:57Z

Yeah, and that's exactly the reason why I was suggesting a two-phase commit
approach: first you ask all the involved listeners "are you ready to commit
to this change?" and if all agree then you propagate the change.
On Jul 30, 2013 8:38 PM, "Luigi R. Viggiano" notifications@github.com
wrote:

That's not the hard, I think.

But there are some cases to take in mind.

Suppose property X is change, and the value passes from A to B.

There are two listeners registered for changes on property X.

1st listener gets notified, and for him X := B.
2nd listener gets notified and throws the exception to say "no, roll back
X := A"
3rd listener doesn't get notified, since the 2nd cancelled the change.

So... we have this situation.
For the 1st listener X is equal to B. For the 2nd and 3rd listeners X is
equal to A...

So... troubles :)

One could send another event to 1st listener to say that X gets assigned
back to A... but... what if then the 1st listener throws an exception? ....
it's a matter to save "capra e cavoli" ;)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/47#issuecomment-21816818
.

lviggiano · 2013-07-31T12:16:21Z

So it can be something like:

cfg.addPropertyChangeListener(new PropertyChangeListener() {
    void beforePropertyChange(PropertyChangeEvent evt) throws Whatever... {
         //....
    }

   void afterPropertyChange(PropertyChangeEvent evt) {  // throws nothing
   }
}

In this way one received the event before the change and have the chance to rollback throwing an exception, but he can only assume that the property is actually changed on the method afterPropertyChange().

Sounds good?

bbossola · 2013-07-31T13:25:38Z

I would probably use a different naming, such as:

cfg.addPropertyChangeListener(new PropertyChangeListener() {
void preparePropertyChange(PropertyChangeEvent evt) throws Whatever... {
//....
}

void commitPropertyChange(PropertyChangeEvent evt) { // throws nothing
//....
}}

It looks more clear to me as it sounds much more like a 2 phase commit:

on preparePropertyChange you have the opportunity to vote NO to a
property change: if you don't then it means you are fully prepared to
commit to this change
on commitPropertyChange you will have to commit the change

The system will first ask to all participant (listeners) to be prepared for
the change: if all say yes, then he commits every one of them.
Cheers,

Bruno

On Wed, Jul 31, 2013 at 1:16 PM, Luigi R. Viggiano <notifications@github.com

wrote:

So it can be something like:

cfg.addPropertyChangeListener(new PropertyChangeListener() {
void beforePropertyChange(PropertyChangeEvent evt) throws Whatever... {
//....
}

void afterPropertyChange(PropertyChangeEvent evt) { // throws nothing
}}

In this way one received the event before the change and have the chance
to rollback throwing an exception, but he can only assume that the property
is actually changed on the method afterPropertyChange().

Sounds good?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/47#issuecomment-21857438
.

vaibhavk81 · 2013-09-06T06:31:36Z

@lviggiano - Can I use master build to detect property change? I just need to know which property was changed in my application.

vaibhavk81 · 2013-09-06T06:38:35Z

@lviggiano - When I build from master, 12 tests are failing. Can I skip the tests and build it?

lviggiano · 2013-09-06T06:44:18Z

The feature is still a work in progress, so, it's not completed yet (mostly unimplemented). I'll work on this asap to complete the feature.

BTW tests on master branch should not be failing. Can you post your build log or send it to me by email?

As far as I know all tests should be green.

See:
https://aeonbits.ci.cloudbees.com/job/owner-api/
https://travis-ci.org/lviggiano/owner
http://sheldon.dyndns.tv:9000/dashboard/index/org.aeonbits.owner:owner

There is one multi-thread test that sometimes fails due to the fact that the test is not perfect synchronizing delays before to check results. Afaik this is the only test that produces build failures, and I need to look into it.

vaibhavk81 · 2013-09-06T06:52:51Z

I just emailed you the logs.
I would really appreciate if you can add this feature asap.

lviggiano · 2013-09-06T06:55:13Z

I'll do. Thanks for the logs. I'm planning next week to start again coding
on this project. Still in holidays.
Il giorno 06/set/2013 08:52, "vaibhavk81" notifications@github.com ha
scritto:

I just emailed you the logs.
I would really appreciate if you can add this feature asap.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/47#issuecomment-23922155
.

lviggiano · 2013-09-11T08:23:30Z

@vaibhavk81 I'm actively working to this feature those days. It should not take long time.
About your logs, I think you have a dirty copy of the master branch, since I am not noticing failing tests; try a fresh checkout.

vaibhavk81 · 2013-09-11T11:01:36Z

Thanks!
I will wait for the new feature and then get the latest from master.

~Vaibhav

On Wed, Sep 11, 2013 at 1:53 PM, Luigi R. Viggiano <notifications@github.com

wrote:

@vaibhavk81 https://github.com/vaibhavk81 I'm actively working to this
feature those days. It should not take long time.
About your logs, I think you have a dirty copy of the master branch, since
I am not noticing failing tests; try a fresh checkout.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/47#issuecomment-24220586
.

lviggiano · 2013-09-12T15:01:07Z

I think the implementation is almost ready, I need to write some more tests and the documentation to close this issue. It's early to say "done", but you can start testing in non-critic environments, if you like.

See Mutable:

    /**
     * Adds a {@link PropertyChangeListener} to the Mutable interface.
     *
     * @param listener the listener to be added.
     * @since 1.0.5
     */
    void addPropertyChangeListener(PropertyChangeListener listener);

PropertyChangeListener comes from the JRE libraries.

I will later add a method to register on a single propertyName:

    void addPropertyChangeListener(String propertyName, PropertyChangeListener listener);

I also extended the PropertyChangeListener with TransactionalPropertyChangeListener that exposes an additional method:

/**
 * A Listener that is aware of properties changes.
 *
 * @author Luigi R. Viggiano
 */
public interface TransactionalPropertyChangeListener extends PropertyChangeListener {

    /**
     * This method is invoked before the property is changed. When this method is invoked we cannot assume that the
     * change is effective, since some listener can ask to roll back the change operation.
     *
     * @param evt the {@link PropertyChangeEvent event} of property change.
     * @throws RollbackOperationException when the listener wants to rollback the change on the property intercepted
     * @throws RollbackBatchException     when the listener wants to rollback the entire set of changes if executed in the
     *                                    batch.
     */
    void beforePropertyChange(PropertyChangeEvent evt) throws RollbackOperationException, RollbackBatchException;

}

Test cases are here:

In particular I need to test more the reload().

lviggiano · 2013-09-13T18:27:02Z

Testing is complete.

Things to do:

method void addPropertyChangeListener(String propertyName, PropertyChangeListener listener);
tests for above method
documentation on the website

@vaibhavk81 if you want to start using the feature, you can now.
Latest jars are here: http://oss.sonatype.org/content/repositories/snapshots/org/aeonbits/owner/owner/1.0.5-SNAPSHOT/
Latest javadocs etc are here: http://owner.newinstance.it/1.0.5-SNAPSHOT/
Good luck ;)

vaibhavk81 · 2013-09-14T09:17:21Z

Thanks a lot!
I will test it out and let you know.

~Vaibhav

On Fri, Sep 13, 2013 at 11:57 PM, Luigi R. Viggiano <
notifications@github.com> wrote:

Testing is complete.

Things to do:

documentation on the website

method void addPropertyChangeListener(String propertyName,
PropertyChangeListener listener);

tests for above method.

@vaibhavk81 https://github.com/vaibhavk81 if you want to start using
the feature, you can now. Good luck ;)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/47#issuecomment-24414668
.

lviggiano · 2013-09-18T14:16:35Z

I completed a draft implementation for PropertyChangeListeners, @bbossola and @vaibhavk81, if you want to have a look at the code or the unit tests regarding the programming interfaces (internals may still change before the 1.0.5 release) you can refer to the unit tests and javadocs.

lviggiano · 2013-10-04T22:23:17Z

@vaibhavk81 I found out that some tests were failing on Windows boxes, since I am testing only on Linux and Mac OS X I didn't notice the problem before.

Current master should build fine on Windows too.

I'm very close to 1.0.5 release which will contain this feature.

lviggiano · 2013-10-07T14:05:15Z

Added documentation page:

http://owner.aeonbits.org/docs/event-support/

Tomorrow I will release 1.0.5

vaibhavk81 · 2013-10-08T18:17:33Z

Thanks a lot!

~Vaibhav

On Mon, Oct 7, 2013 at 7:05 AM, Luigi R. Viggiano
notifications@github.comwrote:

Added documentation page:

http://owner.aeonbits.org/docs/event-support/

Tomorrow I will release 1.0.5

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/47#issuecomment-25810692
.

lviggiano · 2013-10-08T23:21:14Z

@bbossola tomorrow I'll probably release 1.0.5 with this feature implemented. Can you please have a look and review the interfaces for any design suggestion?

http://owner.newinstance.it/1.0.5-SNAPSHOT/xref/index.html
http://owner.newinstance.it/1.0.5-SNAPSHOT/apidocs/index.html

the package is org.aeonbits.owner.event. The PropertyChangeEvent and PropertyChangeListener comes from java.beans.

lviggiano · 2013-10-09T15:14:30Z

1.0.5 released. @bbossola @vaibhavk81 you can update your app with the latest version.

vaibhavk81 · 2013-10-09T15:43:13Z

Will do. Thanks!

Regards,
Vaibhav Khare

Sent from my iPhone

On Oct 9, 2013, at 8:14 AM, "Luigi R. Viggiano" notifications@github.com wrote:

1.0.5 released. @bbossola @vaibhavk81 you can update your app with the latest version.

—
Reply to this email directly or view it on GitHub.

bbossola · 2013-10-09T20:15:28Z

Thx looks cool!
On Oct 9, 2013 4:40 PM, "Luigi R. Viggiano" notifications@github.com
wrote:

1.0.5 released. @bbossola https://github.com/bbossola @vaibhavk81 https://github.com/vaibhavk81you can update your app with the latest version.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/47#issuecomment-25979402
.

ghost assigned lviggiano Sep 12, 2013

lviggiano mentioned this issue Sep 16, 2013

Hot reload does not work when file name needs to be expanded #50

Closed

lviggiano closed this as completed Oct 9, 2013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a method to detect which properties were changed in a reload #47

Add a method to detect which properties were changed in a reload #47

bbossola commented Jul 28, 2013

lviggiano commented Jul 29, 2013

lviggiano commented Jul 29, 2013

lviggiano commented Jul 30, 2013

bbossola commented Jul 30, 2013

lviggiano commented Jul 30, 2013

bbossola commented Jul 31, 2013

lviggiano commented Jul 31, 2013

bbossola commented Jul 31, 2013

vaibhavk81 commented Sep 6, 2013

vaibhavk81 commented Sep 6, 2013

lviggiano commented Sep 6, 2013

vaibhavk81 commented Sep 6, 2013

lviggiano commented Sep 6, 2013

lviggiano commented Sep 11, 2013

vaibhavk81 commented Sep 11, 2013

lviggiano commented Sep 12, 2013

lviggiano commented Sep 13, 2013

vaibhavk81 commented Sep 14, 2013

lviggiano commented Sep 18, 2013

lviggiano commented Oct 4, 2013

lviggiano commented Oct 7, 2013

vaibhavk81 commented Oct 8, 2013

lviggiano commented Oct 8, 2013

lviggiano commented Oct 9, 2013

vaibhavk81 commented Oct 9, 2013

bbossola commented Oct 9, 2013

Add a method to detect which properties were changed in a reload #47

Add a method to detect which properties were changed in a reload #47

Comments

bbossola commented Jul 28, 2013

lviggiano commented Jul 29, 2013

lviggiano commented Jul 29, 2013

lviggiano commented Jul 30, 2013

bbossola commented Jul 30, 2013

lviggiano commented Jul 30, 2013

bbossola commented Jul 31, 2013

lviggiano commented Jul 31, 2013

bbossola commented Jul 31, 2013

vaibhavk81 commented Sep 6, 2013

vaibhavk81 commented Sep 6, 2013

lviggiano commented Sep 6, 2013

vaibhavk81 commented Sep 6, 2013

lviggiano commented Sep 6, 2013

lviggiano commented Sep 11, 2013

vaibhavk81 commented Sep 11, 2013

lviggiano commented Sep 12, 2013

lviggiano commented Sep 13, 2013

vaibhavk81 commented Sep 14, 2013

lviggiano commented Sep 18, 2013

lviggiano commented Oct 4, 2013

lviggiano commented Oct 7, 2013

vaibhavk81 commented Oct 8, 2013

lviggiano commented Oct 8, 2013

lviggiano commented Oct 9, 2013

vaibhavk81 commented Oct 9, 2013

bbossola commented Oct 9, 2013