From 8557bd96b542800ef1ea60780e024eb56d9c52d3 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Wed, 20 Feb 2019 20:36:40 +0100 Subject: [PATCH 1/7] Test refresh() for image sources --- test/spec/ol/source/imagewms.test.js | 51 ++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/test/spec/ol/source/imagewms.test.js b/test/spec/ol/source/imagewms.test.js index d983c6a384..0e228a2534 100644 --- a/test/spec/ol/source/imagewms.test.js +++ b/test/spec/ol/source/imagewms.test.js @@ -1,6 +1,10 @@ import ImageWMS from '../../../../src/ol/source/ImageWMS.js'; +import Image from '../../../../src/ol/layer/Image.js'; import {get as getProjection} from '../../../../src/ol/proj.js'; import {getWidth, getHeight} from '../../../../src/ol/extent.js'; +import View from '../../../../src/ol/View.js'; +import Map from '../../../../src/ol/Map.js'; +import ImageState from '../../../../src/ol/ImageState.js'; describe('ol.source.ImageWMS', function() { @@ -326,4 +330,51 @@ describe('ol.source.ImageWMS', function() { }); }); + describe('#refresh()', function() { + + let map, source; + let callCount = 0; + beforeEach(function(done) { + source = new ImageWMS(options); + source.setImageLoadFunction(function(image) { + ++callCount; + image.state = ImageState.LOADED; + source.loading = false; + }); + const target = document.createElement('div'); + target.style.width = target.style.height = '100px'; + document.body.appendChild(target); + map = new Map({ + target: target, + layers: [ + new Image({ + source: source + }) + ], + view: new View({ + center: [0, 0], + zoom: 0 + }) + }); + map.once('rendercomplete', function() { + callCount = 0; + done(); + }); + }); + + afterEach(function() { + document.body.removeChild(map.getTargetElement()); + map.setTarget(null); + }); + + it('reloads from server', function(done) { + map.once('rendercomplete', function() { + expect(callCount).to.be(1); + done(); + }); + source.refresh(); + }); + + }); + }); From a0ba8dd8c667d07f01b0146e6ea67cbb6de5db74 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Wed, 20 Feb 2019 20:37:22 +0100 Subject: [PATCH 2/7] Add a clear() method for tile sources --- src/ol/source/Tile.js | 12 +++++-- test/spec/ol/source/xyz.test.js | 61 +++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/src/ol/source/Tile.js b/src/ol/source/Tile.js index 9ea34b0ea7..6a21900879 100644 --- a/src/ol/source/Tile.js +++ b/src/ol/source/Tile.js @@ -300,12 +300,20 @@ class TileSource extends Source { return withinExtentAndZ(tileCoord, tileGrid) ? tileCoord : null; } + /** + * Remove all cached tiles from the source. The next render cycle will fetch new tiles. + * @api + */ + clear() { + this.tileCache.clear(); + } + /** * @inheritDoc */ refresh() { - this.tileCache.clear(); - this.changed(); + this.clear(); + super.refresh(); } /** diff --git a/test/spec/ol/source/xyz.test.js b/test/spec/ol/source/xyz.test.js index e59d200516..d6ab713ce6 100644 --- a/test/spec/ol/source/xyz.test.js +++ b/test/spec/ol/source/xyz.test.js @@ -1,8 +1,11 @@ import TileSource from '../../../../src/ol/source/Tile.js'; +import TileLayer from '../../../../src/ol/layer/Tile.js'; import TileImage from '../../../../src/ol/source/TileImage.js'; import UrlTile from '../../../../src/ol/source/UrlTile.js'; import XYZ from '../../../../src/ol/source/XYZ.js'; import {createXYZ} from '../../../../src/ol/tilegrid.js'; +import View from '../../../../src/ol/View.js'; +import Map from '../../../../src/ol/Map.js'; describe('ol.source.XYZ', function() { @@ -183,4 +186,62 @@ describe('ol.source.XYZ', function() { }); + describe('clear and refresh', function() { + + let map, source; + let callCount = 0; + beforeEach(function(done) { + source = new XYZ({ + url: 'spec/ol/data/osm-{z}-{x}-{y}.png', + tileLoadFunction: function(image, src) { + ++callCount; + image.getImage().src = src; + } + }); + const target = document.createElement('div'); + target.style.width = target.style.height = '100px'; + document.body.appendChild(target); + map = new Map({ + target: target, + layers: [ + new TileLayer({ + source: source + }) + ], + view: new View({ + center: [0, 0], + zoom: 0 + }) + }); + map.once('rendercomplete', function() { + callCount = 0; + done(); + }); + }); + + afterEach(function() { + document.body.removeChild(map.getTargetElement()); + map.setTarget(null); + }); + + it('#refresh() reloads from server', function(done) { + map.once('rendercomplete', function() { + expect(callCount).to.be(1); + done(); + }); + source.refresh(); + }); + + it('#clear() clears the tile cache', function(done) { + map.once('rendercomplete', function() { + done(new Error('should not re-render')); + }); + source.clear(); + setTimeout(function() { + done(); + }, 1000); + }); + + }); + }); From f40cbf2cac8b469388fd23e506f7bae15f69d3c6 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Wed, 20 Feb 2019 20:39:04 +0100 Subject: [PATCH 3/7] Do not reload on clear(), but on refresh() --- src/ol/source/Vector.js | 10 ++++- test/spec/ol/source/vector.test.js | 65 ++++++++++++++++++++++++++---- 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/src/ol/source/Vector.js b/src/ol/source/Vector.js index f50aa8d247..f4aeb23360 100644 --- a/src/ol/source/Vector.js +++ b/src/ol/source/Vector.js @@ -498,7 +498,6 @@ class VectorSource extends Source { if (this.featuresRtree_) { this.featuresRtree_.clear(); } - this.loadedExtentsRtree_.clear(); this.nullGeometryFeatures_ = {}; const clearEvent = new VectorSourceEvent(VectorEventType.CLEAR); @@ -894,6 +893,15 @@ class VectorSource extends Source { } } + /** + * @inheritDoc + */ + refresh() { + this.clear(true); + this.loadedExtentsRtree_.clear(); + super.refresh(); + } + /** * Remove an extent from the list of loaded extents. diff --git a/test/spec/ol/source/vector.test.js b/test/spec/ol/source/vector.test.js index 4addb60f42..f00da8a92c 100644 --- a/test/spec/ol/source/vector.test.js +++ b/test/spec/ol/source/vector.test.js @@ -9,6 +9,7 @@ import VectorLayer from '../../../../src/ol/layer/Vector.js'; 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'; describe('ol.source.Vector', function() { @@ -167,8 +168,6 @@ describe('ol.source.Vector', function() { describe('#clear', function() { it('removes all features using fast path', function() { - const changeSpy = sinon.spy(); - listen(vectorSource, 'change', changeSpy); const removeFeatureSpy = sinon.spy(); listen(vectorSource, 'removefeature', removeFeatureSpy); const clearSourceSpy = sinon.spy(); @@ -176,8 +175,6 @@ describe('ol.source.Vector', function() { vectorSource.clear(true); expect(vectorSource.getFeatures()).to.eql([]); expect(vectorSource.isEmpty()).to.be(true); - expect(changeSpy).to.be.called(); - expect(changeSpy.callCount).to.be(1); expect(removeFeatureSpy).not.to.be.called(); expect(removeFeatureSpy.callCount).to.be(0); expect(clearSourceSpy).to.be.called(); @@ -185,8 +182,6 @@ describe('ol.source.Vector', function() { }); it('removes all features using slow path', function() { - const changeSpy = sinon.spy(); - listen(vectorSource, 'change', changeSpy); const removeFeatureSpy = sinon.spy(); listen(vectorSource, 'removefeature', removeFeatureSpy); const clearSourceSpy = sinon.spy(); @@ -194,8 +189,6 @@ describe('ol.source.Vector', function() { vectorSource.clear(); expect(vectorSource.getFeatures()).to.eql([]); expect(vectorSource.isEmpty()).to.be(true); - expect(changeSpy).to.be.called(); - expect(changeSpy.callCount).to.be(1); expect(removeFeatureSpy).to.be.called(); expect(removeFeatureSpy.callCount).to.be(features.length); expect(clearSourceSpy).to.be.called(); @@ -204,6 +197,62 @@ describe('ol.source.Vector', function() { }); + describe('clear and refresh', function() { + + let map, source, spy; + beforeEach(function(done) { + source = new VectorSource({ + format: new GeoJSON(), + url: 'spec/ol/source/vectorsource/single-feature.json' + }); + const target = document.createElement('div'); + target.style.width = target.style.height = '100px'; + document.body.appendChild(target); + map = new Map({ + target: target, + layers: [ + new VectorLayer({ + source: source + }) + ], + view: new View({ + center: [0, 0], + zoom: 0 + }) + }); + map.once('rendercomplete', function() { + spy = sinon.spy(source, 'loader_'); + done(); + }); + }); + + afterEach(function() { + source.loader_.restore(); + document.body.removeChild(map.getTargetElement()); + map.setTarget(null); + }); + + it('#refresh() reloads from server', function(done) { + expect(source.getFeatures()).to.have.length(1); + map.once('rendercomplete', function() { + expect(source.getFeatures()).to.have.length(1); + expect(spy.callCount).to.be(1); + done(); + }); + source.refresh(); + }); + + it('#clear() removes all features from the source', function(done) { + expect(source.getFeatures()).to.have.length(1); + map.once('rendercomplete', function() { + expect(source.getFeatures()).to.have.length(0); + expect(spy.callCount).to.be(0); + done(); + }); + source.clear(); + }); + }); + describe('#forEachFeatureInExtent', function() { it('is called the expected number of times', function() { From 8d1022046e2a6db1593917c4cf12b856e5ac01b4 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Wed, 20 Feb 2019 20:39:26 +0100 Subject: [PATCH 4/7] Clear loaded extents when a new loader is set --- src/ol/source/Vector.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ol/source/Vector.js b/src/ol/source/Vector.js index f4aeb23360..b476191989 100644 --- a/src/ol/source/Vector.js +++ b/src/ol/source/Vector.js @@ -991,6 +991,7 @@ class VectorSource extends Source { * @api */ setLoader(loader) { + this.loadedExtentsRtree_.clear(); this.loader_ = loader; } From e4873a9952553dc400c50da15995bc1378f33722 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Wed, 20 Feb 2019 20:40:09 +0100 Subject: [PATCH 5/7] Improve documentation for ol/Source#refresh --- src/ol/source/Source.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/source/Source.js b/src/ol/source/Source.js index 4d91384514..aeca1ba1bc 100644 --- a/src/ol/source/Source.js +++ b/src/ol/source/Source.js @@ -145,7 +145,7 @@ class Source extends BaseObject { } /** - * Refreshes the source and finally dispatches a 'change' event. + * Refreshes the source. Data from the server will be reloaded. * @api */ refresh() { From 1416a3d162cb6485561390e4d0280fd31053ceb6 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Wed, 20 Feb 2019 20:40:33 +0100 Subject: [PATCH 6/7] Add upgrade notes --- changelog/upgrade-notes.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/changelog/upgrade-notes.md b/changelog/upgrade-notes.md index 817a0cfc49..50f00c1f35 100644 --- a/changelog/upgrade-notes.md +++ b/changelog/upgrade-notes.md @@ -124,6 +124,12 @@ The removed classes and components are: Following the removal of the experimental WebGL renderer, the AtlasManager has been removed as well. The atlas was only used by this renderer. The non API `getChecksum` functions of the style is also removed. +##### Change of the behavior of the vector source's clear() and refresh() methods + +The `ol/source/Vector#clear()` method no longer triggers a reload of the data from the server. If you were previously using `clear()` to refetch from the server, you now have to use `refresh()`. + +The `ol/source/Vector#refresh()` method now triggers a reload of the data from the server. If you were previously using the `refresh()` method to re-render a vector layer, you should instead call `ol/layer/Vector#changed()`. + #### Other changes ##### Allow declutter in image render mode From 94cd126189d1f5615849cdd021ba25ccb2507d8d Mon Sep 17 00:00:00 2001 From: ahocevar Date: Wed, 20 Feb 2019 21:48:08 +0100 Subject: [PATCH 7/7] Add setUrl function and don't reset loaded extents in setLoader --- changelog/upgrade-notes.md | 2 +- src/ol/source/Source.js | 2 +- src/ol/source/Vector.js | 13 ++- test/spec/ol/source/vector.test.js | 131 ++++++++++++++++------------- 4 files changed, 87 insertions(+), 61 deletions(-) diff --git a/changelog/upgrade-notes.md b/changelog/upgrade-notes.md index 50f00c1f35..b03d2f346a 100644 --- a/changelog/upgrade-notes.md +++ b/changelog/upgrade-notes.md @@ -128,7 +128,7 @@ The non API `getChecksum` functions of the style is also removed. The `ol/source/Vector#clear()` method no longer triggers a reload of the data from the server. If you were previously using `clear()` to refetch from the server, you now have to use `refresh()`. -The `ol/source/Vector#refresh()` method now triggers a reload of the data from the server. If you were previously using the `refresh()` method to re-render a vector layer, you should instead call `ol/layer/Vector#changed()`. +The `ol/source/Vector#refresh()` method now removes all features from the source and triggers a reload of the data from the server. If you were previously using the `refresh()` method to re-render a vector layer, you should instead call `ol/layer/Vector#changed()`. #### Other changes diff --git a/src/ol/source/Source.js b/src/ol/source/Source.js index aeca1ba1bc..24dcbf0710 100644 --- a/src/ol/source/Source.js +++ b/src/ol/source/Source.js @@ -145,7 +145,7 @@ class Source extends BaseObject { } /** - * Refreshes the source. Data from the server will be reloaded. + * Refreshes the source. The source will be cleared, and data from the server will be reloaded. * @api */ refresh() { diff --git a/src/ol/source/Vector.js b/src/ol/source/Vector.js index b476191989..b4c26b076f 100644 --- a/src/ol/source/Vector.js +++ b/src/ol/source/Vector.js @@ -985,16 +985,25 @@ class VectorSource extends Source { /** - * Set the new loader of the source. The next loadFeatures call will use the + * Set the new loader of the source. The next render cycle will use the * new loader. * @param {import("../featureloader.js").FeatureLoader} loader The loader to set. * @api */ setLoader(loader) { - this.loadedExtentsRtree_.clear(); this.loader_ = loader; } + /** + * Points the source to a new url. The next render cycle will use the new url. + * @param {string|import("../featureloader.js").FeatureUrlFunction} url Url. + * @api + */ + setUrl(url) { + assert(this.format_, 7); // `format` must be set when `url` is set + this.setLoader(xhr(url, this.format_)); + } + } diff --git a/test/spec/ol/source/vector.test.js b/test/spec/ol/source/vector.test.js index f00da8a92c..fdb8d02f6b 100644 --- a/test/spec/ol/source/vector.test.js +++ b/test/spec/ol/source/vector.test.js @@ -148,6 +148,79 @@ describe('ol.source.Vector', function() { }); + describe('clear and refresh', function() { + + let map, source, spy; + beforeEach(function(done) { + source = new VectorSource({ + format: new GeoJSON(), + url: 'spec/ol/source/vectorsource/single-feature.json' + }); + const target = document.createElement('div'); + target.style.width = target.style.height = '100px'; + document.body.appendChild(target); + map = new Map({ + target: target, + layers: [ + new VectorLayer({ + source: source + }) + ], + view: new View({ + center: [0, 0], + zoom: 0 + }) + }); + map.once('rendercomplete', function() { + spy = sinon.spy(source, 'loader_'); + done(); + }); + }); + + afterEach(function() { + if (spy) { + source.loader_.restore(); + } + document.body.removeChild(map.getTargetElement()); + map.setTarget(null); + }); + + it('#refresh() reloads from server', function(done) { + expect(source.getFeatures()).to.have.length(1); + map.once('rendercomplete', function() { + expect(source.getFeatures()).to.have.length(1); + expect(spy.callCount).to.be(1); + done(); + }); + source.refresh(); + }); + + it('#clear() removes all features from the source', function(done) { + expect(source.getFeatures()).to.have.length(1); + map.once('rendercomplete', function() { + expect(source.getFeatures()).to.have.length(0); + expect(spy.callCount).to.be(0); + done(); + }); + source.clear(); + }); + + it('After #setUrl(), refresh() loads from the new url', function(done) { + source.loader_.restore(); + spy = undefined; + expect(source.getFeatures()).to.have.length(1); + const oldCoordinates = source.getFeatures()[0].getGeometry().getCoordinates(); + map.on('rendercomplete', function() { + expect(source.getFeatures()).to.have.length(1); + const newCoordinates = source.getFeatures()[0].getGeometry().getCoordinates(); + expect(newCoordinates).to.not.eql(oldCoordinates); + done(); + }); + source.setUrl('spec/ol/data/point.json'); + source.refresh(); + }); + }); + describe('when populated with 10 random points and a null', function() { let features; @@ -197,62 +270,6 @@ describe('ol.source.Vector', function() { }); - describe('clear and refresh', function() { - - let map, source, spy; - beforeEach(function(done) { - source = new VectorSource({ - format: new GeoJSON(), - url: 'spec/ol/source/vectorsource/single-feature.json' - }); - const target = document.createElement('div'); - target.style.width = target.style.height = '100px'; - document.body.appendChild(target); - map = new Map({ - target: target, - layers: [ - new VectorLayer({ - source: source - }) - ], - view: new View({ - center: [0, 0], - zoom: 0 - }) - }); - map.once('rendercomplete', function() { - spy = sinon.spy(source, 'loader_'); - done(); - }); - }); - - afterEach(function() { - source.loader_.restore(); - document.body.removeChild(map.getTargetElement()); - map.setTarget(null); - }); - - it('#refresh() reloads from server', function(done) { - expect(source.getFeatures()).to.have.length(1); - map.once('rendercomplete', function() { - expect(source.getFeatures()).to.have.length(1); - expect(spy.callCount).to.be(1); - done(); - }); - source.refresh(); - }); - - it('#clear() removes all features from the source', function(done) { - expect(source.getFeatures()).to.have.length(1); - map.once('rendercomplete', function() { - expect(source.getFeatures()).to.have.length(0); - expect(spy.callCount).to.be(0); - done(); - }); - source.clear(); - }); - }); - describe('#forEachFeatureInExtent', function() { it('is called the expected number of times', function() { @@ -570,7 +587,7 @@ describe('ol.source.Vector', function() { source.loadFeatures([-10000, -10000, 10000, 10000], 1, getProjection('EPSG:3857')); source.setLoader(loader2); - source.clear(); + source.refresh(); source.loadFeatures([-10000, -10000, 10000, 10000], 1, getProjection('EPSG:3857')); expect(count1).to.eql(1);