[patch] filtering the dataset does not generate any events, despite of the fact that the DataSet has actually changed

[cmuelle8]

Code and some plugins listen for DataChangedEvents.

Filtering modifies isDisabled() and isHidden() attributes of OsmPrimitives.
Hence the dataSet may indeed have changed after applying or removing a filter with the FilterDialog.

Symptom:
When a filter is made active, no DataChangedEvent is fired.

Please find attached patch that fires a new DataChangedEvent if the current dataset
has been modified after all current filters executed. It is up to the receiver how
or if this event will be consumed.

[Don-vip]

Replying to cmuelle8:

Filtering modifies isDisabled() and isHidden() attributes of OsmPrimitives.

Then don't we have a PrimitiveFlagsChangedEvent fired?

[cmuelle8]

• FilterModel.java?rev=12691#L107​ executeFilters()
• FilterWorker.java?rev=12656#L53​ doExecuteFilters()
• OsmPrimitive.java?rev=10715#L483​ setDisabledState()
• OsmPrimitive.java?rev=10715#L460​ updateFlagsNoLock()
• AbstractPrimitive.java?rev=12542#L372​ updateFlags()

This is the call chain, none of these methods fires an event, so it seems we do not.
Since filters usually modify a rather large part of the dataset, I guess it won't
replace the necessity to generate and fire a DataChangedEvent.

In the vast majority of cases a filter action will modify more than 30 primitives,
30 being the current maximum in DataSet.java to fire cachedEvents individually.

This means we should not pressure the event queue or the consumer
with one event per primitive each using

• DataSet.java?rev=12537#L1238​ firePrimitiveFlagsChanged()

but instead fire one for the whole collection changed. This will need a class
like PrimitivesFlagsChangedAdded, that constructs with a collection rather
than a single primitive. Firing with a collection will be consistent to how
added and removed primitives are handled, see firePrimitivesAdded()​
and firePrimitivesRemoved()​.

---

So instead of (-I-)

fireEvent(new DataChangedEvent(this));

we would talk about extending it to (-II-)

// flagsChanged is of type Collection<? extends OsmPrimitive>
fireEvent(new DataChangedEvent(this, new PrimitivesFlagsChangedAdded(flagsChanged));

---

If more than 1000 (MAX_EVENTS) individual firePrimitiveFlagsChanged() were
generated instead, all that is fired is a single new DataChangedEvent(this)​.
This means each consumer must provide for the case to scan the whole dataset at all times.

Firing individual events means the consumer would have to listen for and process three different cases:

• individual events
• DataChangedEvent that wraps events (process wrapped)
• DataChangedEvent without payload (process/scan the whole dataset)

Firing one event (either without or with flagsChanged collection payload) the consumer logic
simplifies to processing a single event, which is another reason to avoid using
firePrimitiveFlagsChanged() adding to the one already discussed above.

---

So from the perspective the current code offers, an argument to implement (-II-) instead of
(-I-) may solely be that lots of filter operations modify flags of less than 1000 elements
(in relation to those modifying more).

Bloating event consumer logic for the very rare case that a filter modifies <30 elements,
seems unnecessary / over-elaborate / cumbersome.

Also note that (-I-) paves the way to (-II-), so having (-I-) first, does not hinder
implementation of (-II-) at a later time.

[Don-vip]

In 13208/josm:

fix #15670 - fire a data set change event when applying filters (patch by cmuelle8)

[Don-vip]

In 14206/josm:

fix #16698, see #15670 - make sure filters are executed (costly operation) only when necessary:

• data changes imply execution of filters only when at least a filter is enabled
• filter changes imply execution of filters even is no filter is enabled
• filter dataset change events should not trigger a new filter execution!