Unfortantly we always need a test case when accepting fixes. Still open up
a bug report and it can wait until one of us has time.
Jody Garnett
On Thu, May 8, 2014 at 6:32 AM, Niels Charlier <[email protected]> wrote:
> 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]> 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( FeatureListener listener : source.listeners ){
>> try {
>> listener.changed( notification );
>> }
>> catch (Throwable t ){
>> // 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]> 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(ContentState entry : state.values() ){
>>> if( entry == source ) {
>>> continue; // no notificaiton required
>>> }
>>> for( FeatureListener listener : source.listeners ){
>>> try {
>>> listener.changed( notification );
>>> }
>>> catch (Throwable t ){
>>> // 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(ContentState entry : state.values() ){
>>> if( entry == source ) {
>>> continue; // no notificaiton required
>>> }
>>> for( FeatureListener listener : *entry*.listeners ){
>>> try {
>>> listener.changed( notification );
>>> }
>>> catch (Throwable t ){
>>> // 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]
>>> 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
