Two-way binding between a layer and store in GeoExt.data.store.Features

[geographika]

In GeoExt2 there was two-way binding between a layer and a store - updating a store would update the layer and vice-versa:

        this.on({
            'load': this.onLoad,
            'clear': this.onClear,
            'add': this.onAdd,
            'remove': this.onRemove,
            'update': this.onStoreUpdate,
            scope: this
        });

https://geoext.github.io/geoext2/docs/source/FeatureStore.html

In GeoExt3, from reading the code, the sync is one way - updating a layer will update the store, but removing records from the store will not remove them from the layer:

https://geoext.github.io/geoext3/master/docs/source/Features.html

Is this understanding correct? If so this would be a useful enhancement to the store.

[geographika]

I currently have the following remove listener added to a GeoExt.data.store.Features, which could form the basis of a pull request. It does not currently handle add or modify events.

  listeners: {
      remove: function (store, records) {
          var features = store.getRange(); // get all remaining features
          // currently binding in GeoExt is from layer to store, not store to layer
          // manually remove features linked to records removed from the store
          Ext.each(records, function (r) {
              var feature = r.getFeature();
              var source = store.layer.getSource();
              if (feature) {
                  var featureKey = ol.getUid(feature).toString();
                  // TODO fix this hack
                  // currently a feature can be removed from a layer triggering the
                  // remove event on the grid - which in turn tries to remove the feature
                  // from the layer throwing an error. This check prevents this.
                  if (source.featureChangeKeys_[featureKey]) {
                      source.removeFeature(feature);
                  }
              }
          });
      }
  }

[marcjansen]

Will you habe time to work on a PR?

[geographika]

Probably something to try and implement as part of a code sprint - hopefully in the next couple of months.

Related issues: #489 and #375

[simonseyock]

The feature store extends GeoExt.data.store.OlObjects which expects an ol.Collection to work on. If no collection was provided it creates one.

The olObject store should keep the store and the collection in sync.

If the feature store gets passed a vector layer that has either no source or a source that has no collection, a collection will be provided. But then the collection and the source will not get synchronized.

If an ol.source.Vector will be created with an ol.Collection they will be kept in sync. In my opinion it would be best if we would use that and simply disallow vector sources without collection. I would then throw an error so we could easily see where we need to update the code.

I will add some tests for this and simply check if it works to make sure I am on the right track.

// this will not get synchronized
Ext.create('GeoExt.data.store.Features', {
  layer: new ol.layer.Vector({
    source: new ol.source.Vector()
  })
});

// but this should
Ext.create('GeoExt.data.store.Features', {
  layer: new ol.layer.Vector({
    source: new ol.source.Vector({
      features: new ol.Collection()
    })
  })
});

[simonseyock]

It works as expected. It is a really minimal change in the client code that makes this much easier.

The problem when we would want to properly listen for the vector source is that we effectively a three way binding (store - collection of ol objects store - vector source) and I think it will get messy very fast.

But this enforces the clients of the library to update their code, i.e. is a breaking change.

[simonseyock]

Maybe it would make sense to move the code that keeps the elements of the collection in sync to the ol objects store.

[simonseyock]

@geographika @chrismayer what do you think about the fix I proposed?

#686

[geographika]

Hi @simonseyock. I'm making extensive use of GeoExt.data.store.WfsFeatures which loads features into a store using setData -

geoext/src/data/store/WfsFeatures.js

Line 481 in 6adaf11

me.setData(wfsFeats);

Declarations are currently as follows:

var vectorLayer = new ol.layer.Vector({
    source: new ol.source.Vector(),
    style: style
});

var featStore = Ext.create('GeoExt.data.store.Features', {
    model: featModel,
    layer: vectorLayer,
    filters: filters,
    ...

With the collections approach would the synching work with a simple update as follows?

    source: new ol.source.Vector({
      features: new ol.Collection()
    })

Could this be handled within the store constructor itself?

[simonseyock]

The collection has to be given to the constructor, it can't be set later. So it has to be done before constructing the store. And yes, the update should be enough.

[simonseyock]

I have not tested the setData method to be honest, but it should work. Can you give me example data so I can create an unit test?

[simonseyock]

I looked at it and I think it might be nice to simulate a wfs request in the unit test, so I just would need a sample minimalistic wfs response, then I could write an unit test if you want.

[geographika]

We have a mock GeoJSON response at https://github.com/geoext/geoext/blob/master/test/data/wfs_mock.geojson that is used in a couple of tests e.g. https://github.com/geoext/geoext/blob/f4c4b252d13e6737f4c4da9437eee09067132327/test/spec/GeoExt/data/store/WfsFeatures.test.js
Could this be used?

[simonseyock]

I added some tests for the wfs features store and it works as expected.
https://github.com/geoext/geoext/pull/686/files#diff-d579c2d9e0ddfaeba703a943e65f0d0e98519c0548ff2417ded898952490c775R266-R340

[geographika]

@simonseyock - I'm not sure if you've been able to test this, but what happens if a store is filtered? Would the vector layer also be filtered, or is filtering simply hiding rather than removing features from a store?

[simonseyock]

The features will be removed from the layer. This is like before. Do you think it would be better to hide them?

[geographika]

Removing sounds good to me. If the filters are subsequently cleared then I presume the features would then be readded to the layer?

[simonseyock]

Yes that is correct. On each filter change it is ensured that only the features corresponding to the record entries are in the layer/collection.

I did not check that specifically, but the existing test does work.

The option passThroughFilter has to be set to true, though (like before).