diff --git a/src/ol/source/Vector.js b/src/ol/source/Vector.js index 23aa25a30c..9f4a0f39f5 100644 --- a/src/ol/source/Vector.js +++ b/src/ol/source/Vector.js @@ -241,11 +241,11 @@ class VectorSource extends Source { this.idIndex_ = {}; /** - * A lookup of features without id (keyed by getUid(feature)). + * A lookup of features by uid (using getUid(feature)). * @private * @type {!Object>} */ - this.undefIdIndex_ = {}; + this.uidIndex_ = {}; /** * @private @@ -359,10 +359,11 @@ class VectorSource extends Source { } else { valid = false; } - } else { - assert(!(featureKey in this.undefIdIndex_), + } + if (valid) { + assert(!(featureKey in this.uidIndex_), 30); // The passed `feature` was already added to the source - this.undefIdIndex_[featureKey] = feature; + this.uidIndex_[featureKey] = feature; } return valid; } @@ -489,7 +490,7 @@ class VectorSource extends Source { if (!this.featuresCollection_) { this.featureChangeKeys_ = {}; this.idIndex_ = {}; - this.undefIdIndex_ = {}; + this.uidIndex_ = {}; } } else { if (this.featuresRtree_) { @@ -770,6 +771,18 @@ class VectorSource extends Source { } + /** + * Get a feature by its internal unique identifier (using `getUid`). + * + * @param {string} uid Feature identifier. + * @return {import("../Feature.js").default} The feature (or `null` if not found). + */ + getFeatureByUid(uid) { + const feature = this.uidIndex_[uid]; + return feature !== undefined ? feature : null; + } + + /** * Get the format associated with this source. * @@ -831,20 +844,13 @@ class VectorSource extends Source { const id = feature.getId(); if (id !== undefined) { const sid = id.toString(); - if (featureKey in this.undefIdIndex_) { - delete this.undefIdIndex_[featureKey]; + if (this.idIndex_[sid] !== feature) { + this.removeFromIdIndex_(feature); this.idIndex_[sid] = feature; - } else { - if (this.idIndex_[sid] !== feature) { - this.removeFromIdIndex_(feature); - this.idIndex_[sid] = feature; - } } } else { - if (!(featureKey in this.undefIdIndex_)) { - this.removeFromIdIndex_(feature); - this.undefIdIndex_[featureKey] = feature; - } + this.removeFromIdIndex_(feature); + this.uidIndex_[featureKey] = feature; } this.changed(); this.dispatchEvent(new VectorSourceEvent( @@ -862,7 +868,7 @@ class VectorSource extends Source { if (id !== undefined) { return id in this.idIndex_; } else { - return getUid(feature) in this.undefIdIndex_; + return getUid(feature) in this.uidIndex_; } } @@ -964,9 +970,8 @@ class VectorSource extends Source { const id = feature.getId(); if (id !== undefined) { delete this.idIndex_[id.toString()]; - } else { - delete this.undefIdIndex_[featureKey]; } + delete this.uidIndex_[featureKey]; this.dispatchEvent(new VectorSourceEvent( VectorEventType.REMOVEFEATURE, feature)); } diff --git a/test/spec/ol/source/vector.test.js b/test/spec/ol/source/vector.test.js index d48d2962a3..9fdb56a983 100644 --- a/test/spec/ol/source/vector.test.js +++ b/test/spec/ol/source/vector.test.js @@ -10,6 +10,7 @@ import {bbox as bboxStrategy} from '../../../../src/ol/loadingstrategy.js'; import {get as getProjection, transformExtent, fromLonLat} from '../../../../src/ol/proj.js'; import VectorSource from '../../../../src/ol/source/Vector.js'; import GeoJSON from '../../../../src/ol/format/GeoJSON.js'; +import {getUid} from '../../../../src/ol/util.js'; describe('ol.source.Vector', function() { @@ -519,6 +520,59 @@ describe('ol.source.Vector', function() { }); + describe('#getFeatureByUid()', function() { + let source; + beforeEach(function() { + source = new VectorSource(); + }); + + it('returns a feature with an id', function() { + const feature = new Feature(); + feature.setId('abcd'); + source.addFeature(feature); + expect(source.getFeatureByUid(getUid(feature))).to.be(feature); + }); + + it('returns a feature without id', function() { + const feature = new Feature(); + source.addFeature(feature); + expect(source.getFeatureByUid(getUid(feature))).to.be(feature); + }); + + it('returns null when no feature is found', function() { + const feature = new Feature(); + feature.setId('abcd'); + source.addFeature(feature); + const wrongId = 'abcd'; + expect(source.getFeatureByUid(wrongId)).to.be(null); + }); + + it('returns null after removing feature', function() { + const feature = new Feature(); + feature.setId('abcd'); + source.addFeature(feature); + const uid = getUid(feature); + expect(source.getFeatureByUid(uid)).to.be(feature); + source.removeFeature(feature); + expect(source.getFeatureByUid(uid)).to.be(null); + }); + + it('returns null after clear', function() { + const feature = new Feature(); + feature.setId('abcd'); + source.addFeature(feature); + const uid = getUid(feature); + expect(source.getFeatureByUid(uid)).to.be(feature); + source.clear(); + expect(source.getFeatureByUid(uid)).to.be(null); + }); + + it('returns null when no features are present', function() { + expect(source.getFeatureByUid('abcd')).to.be(null); + }); + + }); + describe('#loadFeatures', function() { describe('with the "bbox" strategy', function() {