Hi Jody,
I understand, cool.
I did test that the change didn't break anything, including online tests.
I wanted to report what seems to be a bug to me.
Writing additional tests for this I think is out of scope of my current
project.
Perhaps in my own time I will do it.
Kind Regards
Niels
On 08/05/14 05:31, Jody Garnett wrote:
Your original bug report made sense, I was just going through the
senario to use to test if fix works.
Jody Garnett
On Wed, May 7, 2014 at 10:52 AM, Niels Charlier <[email protected]
<mailto:[email protected]>> wrote:
Jody,
If the events need to be sent to the listeners of 'source', then
there is no need for a loop. While multiply the event sent to
source by the amount of entries there are (apart from source itself)?
If it does need to be sent to 'source', why the check
if(entry==source){continue;// no notificaiton required}
Basically, the method as it exists today could be simplified as
follows:
for(int i=0; i <state.values().size()-1; i++){
for(FeatureListenerlistener:source.listeners){
try{
listener.changed(notification);
}
catch(Throwablet){
// problem issuing notification to an interested party
dataStore.LOGGER.log(Level.WARNING,"Problem issuing feature event
"+notification,t);
}
}
Now does that make sense?
The thing is, the events have already sent to source by source
itself, it is source who calls this method after sending the
events to its own listeners. It seems to me that is why the
'continue' check is in place... because it needs to be sent to
entries /other/ than source.
Kind Regards
Niels
On 07/05/14 15:32, Jody Garnett wrote:
I am not sure Niels, we do have some tricky logic around source
vs entry, but I expect you are on to something.
The test case to write (to confirm this) involves two transactions:
a) One transaction issuing a bulk event, such as commit
b) A second transaction, which should get this commit
notification sent to its listeners
The other case is transaction auto commit:
a) event sent on transaction auto commit, such as feature add
b) a second transaction, which should get this feature add event
notification
Jody
Jody Garnett
On Wed, May 7, 2014 at 7:15 AM, Niels Charlier <[email protected]
<mailto:[email protected]>> wrote:
Hi Everyone,
while working on wfs-ng, I think I found a bug in the events
system in gt-data.
This is my suggested change:
https://github.com/NielsCharlier/geotools/commit/af9df3e282d0ad70fbf463fa5e2b36225a53116a
In
modules/library/data/src/main/java/org/geotools/data/store/ContentEntry.java
, now it is:
for(ContentStateentry:state.values()){
if(entry==source){
continue;// no notificaiton required
}
for(FeatureListenerlistener:source.listeners){
try{
listener.changed(notification);
}
catch(Throwablet){
// problem issuing notification to an interested party
dataStore.LOGGER.log(Level.WARNING,"Problem issuing feature
event "+notification,t);
}
}
}
I would assume it should be this:
for(ContentStateentry:state.values()){
if(entry==source){
continue;// no notificaiton required
}
for(FeatureListenerlistener:*entry*.listeners){
try{
listener.changed(notification);
}
catch(Throwablet){
// problem issuing notification to an interested party
dataStore.LOGGER.log(Level.WARNING,"Problem issuing feature
event "+notification,t);
}
}
}
Why loop over the entries otherwise? It seems to me my change
makes more sense.
Kind Regards
Niels
------------------------------------------------------------------------------
Is your legacy SCM system holding you back? Join Perforce May
7 to find out:
• 3 signs your SCM is hindering your productivity
• Requirements for releasing software faster
• Expert tips and advice for migrating your SCM now
http://p.sf.net/sfu/perforce
_______________________________________________
GeoTools-Devel mailing list
[email protected]
<mailto:[email protected]>
https://lists.sourceforge.net/lists/listinfo/geotools-devel
------------------------------------------------------------------------------
Is your legacy SCM system holding you back? Join Perforce May 7 to find out:
• 3 signs your SCM is hindering your productivity
• Requirements for releasing software faster
• Expert tips and advice for migrating your SCM now
http://p.sf.net/sfu/perforce
_______________________________________________
GeoTools-Devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
