Add method for retrieving ol.Overlay by id

externs/olx.js

@@ -309,7 +309,8 @@ olx.MapOptions.prototype.view;
 
 /**
  * Object literal with config options for the overlay.
- * @typedef {{element: (Element|undefined),
+ * @typedef {{id: (number|string|undefined),
+ *     element: (Element|undefined),
  *     offset: (Array.<number>|undefined),
  *     position: (ol.Coordinate|undefined),
  *     positioning: (ol.OverlayPositioning|string|undefined),
@@ -323,6 +324,15 @@ olx.MapOptions.prototype.view;
 olx.OverlayOptions;
 
 
+/**
+ * Set the overlay id. The overlay id can be used with the
+ * {@link ol.Map#getOverlayById} method.
+ * @type {number|string|undefined}
+ * @api
+ */
+olx.OverlayOptions.prototype.id;
+
+
 /**
  * The overlay element.
  * @type {Element|undefined}

src/ol/map.js

@@ -557,9 +557,6 @@ ol.Map.prototype.addOverlayInternal = function(overlay) {
     this.overlayIdIndex_[id.toString()] = overlay;
   }
   overlay.setMap(this);
-  goog.events.listen(
-      overlay, ol.Object.getChangeEventType(overlay.getOverlayIdProperty()),
-      this.handleOverlayIdChange_, false, this);
 };
 
 
@@ -1103,28 +1100,6 @@ ol.Map.prototype.handleTileChange_ = function() {
 };
 
 
-/**
- * @param {goog.events.Event} event Event.
- * @private
- */
-ol.Map.prototype.handleOverlayIdChange_ = function(event) {
-  var overlay = /** @type {ol.Overlay} */ (event.target);
-  var id = overlay.getId().toString();
-  var oldId = event.oldValue;
-  if (oldId && oldId != id) {
-    delete this.overlayIdIndex_[oldId];
-  }
-  if (id in this.overlayIdIndex_) {
-    if (this.overlayIdIndex_[id] !== overlay) {
-      delete this.overlayIdIndex_[id];
-      this.overlayIdIndex_[id] = overlay;
-    }
-  } else {
-    this.overlayIdIndex_[id] = overlay;
-  }
-};
-
-
 /**
  * @private
  */

src/ol/overlay.js

@@ -19,7 +19,6 @@ goog.require('ol.extent');
  * @enum {string}
  */
 ol.OverlayProperty = {
-  ID: 'id',
   ELEMENT: 'element',
   MAP: 'map',
   OFFSET: 'offset',
@@ -78,7 +77,7 @@ ol.Overlay = function(options) {
    * @private
    * @type {number|string|undefined}
    */
-  this.id_ = undefined;
+  this.id_ = options.id;
 
   /**
    * @private
@@ -195,8 +194,7 @@ ol.Overlay.prototype.getElement = function() {
 
 
 /**
- * Get the feature identifier.  This is an identifier for the overlay and
+ * Get the feature identifier which is set on constructor.
- * is set explicitly by calling {@link ol.Overlay#setId}.
  * @return {number|string|undefined} Id.
  * @api
  */
@@ -229,15 +227,6 @@ ol.Overlay.prototype.getOffset = function() {
 };
 
 
-/**
- * Workaround to overcome circular dependency.
- * @return {ol.OverlayProperty}
- */
-ol.Overlay.prototype.getOverlayIdProperty = function() {
-  return ol.OverlayProperty.ID;
-};
-
-
 /**
  * Get the current position of this overlay.
  * @return {ol.Coordinate|undefined} The spatial point that the overlay is
@@ -348,22 +337,6 @@ ol.Overlay.prototype.setElement = function(element) {
 };
 
 
-/**
- * Set the feature id. The feature id can be used with the
- * {@link ol.Map#getOverlayById} method.
- * @param {number|string} id The feature id.
- * @observable
- * @api
- */
-ol.Overlay.prototype.setId = function(id) {
-  goog.asserts.assert(id !== undefined, 'overlay id should be defined');
-  if (id != this.id_) {
-    this.id_ = id;
-    this.set(ol.OverlayProperty.ID, id);
-  }
-};
-
-
 /**
  * Set the map to be associated with this overlay.
  * @param {ol.Map|undefined} map The map that the overlay is part of.

test/spec/ol/map.test.js

@@ -346,80 +346,36 @@ describe('ol.Map', function() {
 
       it('returns an overlay by id', function() {
         overlay = new ol.Overlay({
-          element: overlay_target,
+          id: 'foo',
-          position: [0, 0]
-        });
-        overlay.setId('foo');
-        map.addOverlay(overlay);
-        expect(map.getOverlayById('foo')).to.be(overlay);
-      });
-
-      it('returns an overlay by id (set after add)', function() {
-        overlay = new ol.Overlay({
           element: overlay_target,
           position: [0, 0]
         });
         map.addOverlay(overlay);
-        expect(map.getOverlayById('foo')).to.be(null);
-        overlay.setId('foo');
         expect(map.getOverlayById('foo')).to.be(overlay);
       });
 
       it('returns null when no overlay is found', function() {
         overlay = new ol.Overlay({
+          id: 'foo',
           element: overlay_target,
           position: [0, 0]
         });
-        overlay.setId('foo');
         map.addOverlay(overlay);
         expect(map.getOverlayById('bar')).to.be(null);
       });
 
       it('returns null after removing overlay', function() {
         overlay = new ol.Overlay({
+          id: 'foo',
           element: overlay_target,
           position: [0, 0]
         });
-        overlay.setId('foo');
         map.addOverlay(overlay);
         expect(map.getOverlayById('foo')).to.be(overlay);
         map.removeOverlay(overlay);
         expect(map.getOverlayById('foo')).to.be(null);
       });
 
-      it('returns correct overlay after add/remove/add', function() {
-        expect(map.getOverlayById('foo')).to.be(null);
-        var first = new ol.Overlay({
-          element: overlay_target,
-          position: [0, 0]
-        });
-        first.setId('foo');
-        map.addOverlay(first);
-        expect(map.getOverlayById('foo')).to.be(first);
-        map.removeOverlay(first);
-        expect(map.getOverlayById('foo')).to.be(null);
-        var second = new ol.Overlay({
-          element: overlay_target,
-          position: [0, 0]
-        });
-        second.setId('foo');
-        map.addOverlay(second);
-        expect(map.getOverlayById('foo')).to.be(second);
-      });
-
-      it('returns correct overlay after add/change', function() {
-        expect(map.getOverlayById('foo')).to.be(null);
-        overlay = new ol.Overlay({
-          element: overlay_target,
-          position: [0, 0]
-        });
-        overlay.setId('foo');
-        map.addOverlay(overlay);
-        expect(map.getOverlayById('foo')).to.be(overlay);
-        overlay.setId('bar');
-        expect(map.getOverlayById('foo')).to.be(null);
-        expect(map.getOverlayById('bar')).to.be(overlay);
-      });
     });
 
   });

test/spec/ol/overlay.test.js

@@ -41,34 +41,33 @@ describe('ol.Overlay', function() {
 
   });
 
-  describe('#setId()', function() {
+  describe('#getId()', function() {
     var overlay, target;
 
     beforeEach(function() {
       target = document.createElement('div');
-      overlay = new ol.Overlay({
-        element: target,
-        position: [0, 0]
-      });
-      map.addOverlay(overlay);
     });
     afterEach(function() {
       map.removeOverlay(overlay);
     });
 
-    it('sets the overlay identifier', function() {
+    it('returns the overlay identifier', function() {
+      overlay = new ol.Overlay({
+        element: target,
+        position: [0, 0]
+      });
+      map.addOverlay(overlay);
       expect(overlay.getId()).to.be(undefined);
-      overlay.setId('foo');
+      map.removeOverlay(overlay);
+      overlay = new ol.Overlay({
+        id: 'foo',
+        element: target,
+        position: [0, 0]
+      });
+      map.addOverlay(overlay);
       expect(overlay.getId()).to.be('foo');
     });
 
-    it('accepts a string or number', function() {
-      overlay.setId('foo');
-      expect(overlay.getId()).to.be('foo');
-      overlay.setId(2);
-      expect(overlay.getId()).to.be(2);
-    });
-
   });
 
 });