From 34cabd1579e526407b386d402e9dd742dc34c383 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Tue, 20 May 2014 09:35:14 -0600 Subject: [PATCH 1/3] Dispatch change on feature id change --- src/ol/feature.js | 1 + test/spec/ol/feature.test.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/ol/feature.js b/src/ol/feature.js index a0909998dd..553d0c39d5 100644 --- a/src/ol/feature.js +++ b/src/ol/feature.js @@ -185,6 +185,7 @@ ol.Feature.prototype.setStyle = function(style) { */ ol.Feature.prototype.setId = function(id) { this.id_ = id; + this.dispatchChangeEvent(); }; diff --git a/test/spec/ol/feature.test.js b/test/spec/ol/feature.test.js index de7d617344..76f5ea4d69 100644 --- a/test/spec/ol/feature.test.js +++ b/test/spec/ol/feature.test.js @@ -232,6 +232,34 @@ describe('ol.Feature', function() { }); + describe('#setId()', function() { + + it('sets the feature identifier', function() { + var feature = new ol.Feature(); + expect(feature.getId()).to.be(undefined); + feature.setId('foo'); + expect(feature.getId()).to.be('foo'); + }); + + it('accepts a string or number', function() { + var feature = new ol.Feature(); + feature.setId('foo'); + expect(feature.getId()).to.be('foo'); + feature.setId(2); + expect(feature.getId()).to.be(2); + }); + + it('dispatches the "change" event', function(done) { + var feature = new ol.Feature(); + feature.on('change', function() { + expect(feature.getId()).to.be('foo'); + done(); + }); + feature.setId('foo'); + }); + + }); + describe('#getStyleFunction()', function() { var styleFunction = function(resolution) { From 652f11cefa5352eabc63eab3afe7349c6aad057c Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Tue, 20 May 2014 09:37:36 -0600 Subject: [PATCH 2/3] Provide a method for retrieving features by id --- src/ol/source/vectorsource.js | 91 +++++++++++++++++ test/spec/ol/source/vectorsource.test.js | 119 +++++++++++++++++++++++ 2 files changed, 210 insertions(+) diff --git a/src/ol/source/vectorsource.js b/src/ol/source/vectorsource.js index 066a09cdd4..50a7eca76b 100644 --- a/src/ol/source/vectorsource.js +++ b/src/ol/source/vectorsource.js @@ -69,6 +69,20 @@ ol.source.Vector = function(opt_options) { */ this.nullGeometryFeatures_ = {}; + /** + * A lookup of features by id (the return from feature.getId()). + * @private + * @type {Object.} + */ + this.idIndex_ = {}; + + /** + * A lookup of features without id (keyed by goog.getUid(feature)). + * @private + * @type {Object.} + */ + this.undefIdIndex_ = {}; + /** * @private * @type {Object.>} @@ -116,6 +130,14 @@ ol.source.Vector.prototype.addFeatureInternal = function(feature) { } else { this.nullGeometryFeatures_[goog.getUid(feature).toString()] = feature; } + var id = feature.getId(); + if (goog.isDef(id)) { + var sid = id.toString(); + goog.asserts.assert(!(goog.isDef(this.idIndex_[sid]))); + this.idIndex_[sid] = feature; + } else { + this.undefIdIndex_[goog.getUid(feature).toString()] = feature; + } this.dispatchEvent( new ol.source.VectorEvent(ol.source.VectorEventType.ADDFEATURE, feature)); }; @@ -314,6 +336,20 @@ ol.source.Vector.prototype.getExtent = function() { }; +/** + * Get a feature by its identifier (the value returned by feature.getId()). + * Note that the index treats string and numeric identifiers as the same. So + * `source.getFeatureById(2)` will return a feature with id `'2'` or `2`. + * + * @param {string|number} id Feature identifier. + * @return {ol.Feature} The feature (or `null` if not found). + */ +ol.source.Vector.prototype.getFeatureById = function(id) { + var feature = this.idIndex_[id.toString()]; + return goog.isDef(feature) ? feature : null; +}; + + /** * @param {goog.events.Event} event Event. * @private @@ -336,6 +372,35 @@ ol.source.Vector.prototype.handleFeatureChange_ = function(event) { this.rBush_.update(extent, feature); } } + var id = feature.getId(); + var removed; + if (goog.isDef(id)) { + var sid = id.toString(); + if (featureKey in this.undefIdIndex_) { + delete this.undefIdIndex_[featureKey]; + goog.asserts.assert(!goog.isDef(this.idIndex_[sid]), + 'Duplicate feature id: ' + id); + this.idIndex_[sid] = feature; + } else { + if (this.idIndex_[sid] !== feature) { + removed = this.removeFromIdIndex_(feature); + goog.asserts.assert(removed, + 'Expected feature to be removed from index'); + goog.asserts.assert(!(sid in this.idIndex_), + 'Duplicate feature id: ' + id); + this.idIndex_[sid] = feature; + } + } + } else { + if (!(featureKey in this.undefIdIndex_)) { + removed = this.removeFromIdIndex_(feature); + goog.asserts.assert(removed, + 'Expected feature to be removed from index'); + this.undefIdIndex_[featureKey] = feature; + } else { + goog.asserts.assert(this.undefIdIndex_[featureKey] === feature); + } + } this.dispatchChangeEvent(); }; @@ -384,11 +449,37 @@ ol.source.Vector.prototype.removeFeatureInternal = function(feature) { goog.array.forEach(this.featureChangeKeys_[featureKey], goog.events.unlistenByKey); delete this.featureChangeKeys_[featureKey]; + var id = feature.getId(); + if (goog.isDef(id)) { + delete this.idIndex_[id.toString()]; + } else { + delete this.undefIdIndex_[featureKey]; + } this.dispatchEvent(new ol.source.VectorEvent( ol.source.VectorEventType.REMOVEFEATURE, feature)); }; +/** + * Remove a feature from the id index. Called internally when the feature id + * may have changed. + * @param {ol.Feature} feature The feature. + * @return {boolean} Removed the feature from the index. + * @private + */ +ol.source.Vector.prototype.removeFromIdIndex_ = function(feature) { + var removed = false; + for (var id in this.idIndex_) { + if (this.idIndex_[id] === feature) { + delete this.idIndex_[id]; + removed = true; + break; + } + } + return removed; +}; + + /** * @constructor diff --git a/test/spec/ol/source/vectorsource.test.js b/test/spec/ol/source/vectorsource.test.js index 0b726799b9..3777c1fd83 100644 --- a/test/spec/ol/source/vectorsource.test.js +++ b/test/spec/ol/source/vectorsource.test.js @@ -244,6 +244,125 @@ describe('ol.source.Vector', function() { }); + describe('#getFeatureById()', function() { + var source; + beforeEach(function() { + source = new ol.source.Vector(); + }); + + it('returns a feature by id', function() { + var feature = new ol.Feature(); + feature.setId('foo'); + source.addFeature(feature); + expect(source.getFeatureById('foo')).to.be(feature); + }); + + it('returns a feature by id (set after add)', function() { + var feature = new ol.Feature(); + source.addFeature(feature); + expect(source.getFeatureById('foo')).to.be(null); + feature.setId('foo'); + expect(source.getFeatureById('foo')).to.be(feature); + }); + + it('returns null when no feature is found', function() { + var feature = new ol.Feature(); + feature.setId('foo'); + source.addFeature(feature); + expect(source.getFeatureById('bar')).to.be(null); + }); + + it('returns null after removing feature', function() { + var feature = new ol.Feature(); + feature.setId('foo'); + source.addFeature(feature); + expect(source.getFeatureById('foo')).to.be(feature); + source.removeFeature(feature); + expect(source.getFeatureById('foo')).to.be(null); + }); + + it('returns null after unsetting id', function() { + var feature = new ol.Feature(); + feature.setId('foo'); + source.addFeature(feature); + expect(source.getFeatureById('foo')).to.be(feature); + feature.setId(undefined); + expect(source.getFeatureById('foo')).to.be(null); + }); + + it('returns null after clear', function() { + var feature = new ol.Feature(); + feature.setId('foo'); + source.addFeature(feature); + expect(source.getFeatureById('foo')).to.be(feature); + source.clear(); + expect(source.getFeatureById('foo')).to.be(null); + }); + + it('returns null when no features are indexed', function() { + expect(source.getFeatureById('foo')).to.be(null); + source.addFeature(new ol.Feature()); + expect(source.getFeatureById('foo')).to.be(null); + }); + + it('returns correct feature after add/remove/add', function() { + expect(source.getFeatureById('foo')).to.be(null); + var first = new ol.Feature(); + first.setId('foo'); + source.addFeature(first); + expect(source.getFeatureById('foo')).to.be(first); + source.removeFeature(first); + expect(source.getFeatureById('foo')).to.be(null); + var second = new ol.Feature(); + second.setId('foo'); + source.addFeature(second); + expect(source.getFeatureById('foo')).to.be(second); + }); + + it('returns correct feature after add/change', function() { + expect(source.getFeatureById('foo')).to.be(null); + var feature = new ol.Feature(); + feature.setId('foo'); + source.addFeature(feature); + expect(source.getFeatureById('foo')).to.be(feature); + feature.setId('bar'); + expect(source.getFeatureById('foo')).to.be(null); + expect(source.getFeatureById('bar')).to.be(feature); + }); + + }); + + describe('the feature id index', function() { + var source; + beforeEach(function() { + source = new ol.source.Vector(); + }); + + it('enforces a uniqueness constraint (on add)', function() { + var feature = new ol.Feature(); + feature.setId('foo'); + source.addFeature(feature); + var dupe = new ol.Feature(); + dupe.setId('foo'); + expect(function() { + source.addFeature(dupe); + }).to.throwException(); + }); + + it('enforces a uniqueness constraint (on change)', function() { + var foo = new ol.Feature(); + foo.setId('foo'); + source.addFeature(foo); + var bar = new ol.Feature(); + bar.setId('bar'); + source.addFeature(bar); + expect(function() { + bar.setId('foo'); + }).to.throwException(); + }); + + }); + }); From a2b81d6bd0a4d0b2a9004baaa8a2854115abf99b Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Tue, 20 May 2014 10:05:47 -0600 Subject: [PATCH 3/3] Disallow adding the same feature twice --- src/ol/source/vectorsource.js | 9 ++++++--- test/spec/ol/source/vectorsource.test.js | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/ol/source/vectorsource.js b/src/ol/source/vectorsource.js index 50a7eca76b..03b4ffb3b6 100644 --- a/src/ol/source/vectorsource.js +++ b/src/ol/source/vectorsource.js @@ -128,15 +128,18 @@ ol.source.Vector.prototype.addFeatureInternal = function(feature) { var extent = geometry.getExtent(); this.rBush_.insert(extent, feature); } else { - this.nullGeometryFeatures_[goog.getUid(feature).toString()] = feature; + this.nullGeometryFeatures_[featureKey] = feature; } var id = feature.getId(); if (goog.isDef(id)) { var sid = id.toString(); - goog.asserts.assert(!(goog.isDef(this.idIndex_[sid]))); + goog.asserts.assert(!(sid in this.idIndex_), + 'Feature with same id already added to the source: ' + id); this.idIndex_[sid] = feature; } else { - this.undefIdIndex_[goog.getUid(feature).toString()] = feature; + goog.asserts.assert(!(featureKey in this.undefIdIndex_), + 'Feature already added to the source'); + this.undefIdIndex_[featureKey] = feature; } this.dispatchEvent( new ol.source.VectorEvent(ol.source.VectorEventType.ADDFEATURE, feature)); diff --git a/test/spec/ol/source/vectorsource.test.js b/test/spec/ol/source/vectorsource.test.js index 3777c1fd83..8957fd2305 100644 --- a/test/spec/ol/source/vectorsource.test.js +++ b/test/spec/ol/source/vectorsource.test.js @@ -363,6 +363,21 @@ describe('ol.source.Vector', function() { }); + describe('the undefined feature id index', function() { + var source; + beforeEach(function() { + source = new ol.source.Vector(); + }); + + it('disallows adding the same feature twice', function() { + var feature = new ol.Feature(); + source.addFeature(feature); + expect(function() { + source.addFeature(feature); + }).to.throwException(); + }); + }); + });