Skip to content

Commit

Permalink
Make CesiumGeohash compatible with new methods +
Browse files Browse the repository at this point in the history
Other minor fixes and improvements:
- Add configurable collection-wide min and max precision values in Geohashes (set max to 9 rather than 6)
- Always validate precisions and bounds in methods that use them in Geohashes collection
- Account for bounding boxes that cross the antimeridian or have north < south when calculating area
- Fix issue where the search results remained in "loading" state when the search was cancelled (e.g. the request URL didn't change, happens when the Cesium camera view has not changed sufficiently)
- Decrease limit of number of geohashes in spatial filter to respect Solr's boolean clauses limit
- Fix issue where pager was hidden when there were 25-50 results
- Remove old CesiumGeohashes view
- Remove unused methods from CesiumGeohash model & Geohashes collection

Issues: #2119, #1720
  • Loading branch information
robyngit committed May 4, 2023
1 parent fa9584b commit be0f061
Show file tree
Hide file tree
Showing 9 changed files with 290 additions and 289 deletions.
268 changes: 155 additions & 113 deletions src/js/collections/maps/Geohashes.js

Large diffs are not rendered by default.

11 changes: 10 additions & 1 deletion src/js/models/connectors/Filters-Map.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ define(["backbone", "collections/Filters", "models/maps/Map"], function (
* @property {boolean} isConnected Whether the connector is currently
* listening to the Map model for changes. Set automatically when the
* connector is started or stopped.
* @property {function} onMoveEnd A function to call when the map is
* finished moving. This function will be called with the connector as
* 'this'.
* @since x.x.x
*/
defaults: function () {
Expand All @@ -41,6 +44,7 @@ define(["backbone", "collections/Filters", "models/maps/Map"], function (
spatialFilters: [],
map: new Map(),
isConnected: false,
onMoveEnd: this.updateSpatialFilters,
};
},

Expand Down Expand Up @@ -187,7 +191,12 @@ define(["backbone", "collections/Filters", "models/maps/Map"], function (
this.listenTo(map, "moveStart", function () {
this.get("filters").trigger("changing");
});
this.listenTo(map, "moveEnd", this.updateSpatialFilters);
this.listenTo(map, "moveEnd", function () {
const moveEndFunc = this.get("onMoveEnd");
if (typeof moveEndFunc === "function") {
moveEndFunc.call(this);
}
});
this.set("isConnected", true);
} catch (e) {
console.log("Error starting Filter-Map listeners: ", e);
Expand Down
3 changes: 3 additions & 0 deletions src/js/models/connectors/Filters-Search.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,9 @@ define([
// do anything. This function may have been triggered by a change event
// on a filter that doesn't affect the query at all
if (!searchResults.hasChanged()) {
// Trigger a reset event to indicate the search is complete (e.g. for
// the UI)
searchResults.trigger("reset");
return;
}

Expand Down
67 changes: 64 additions & 3 deletions src/js/models/connectors/Map-Search-Filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ define([
* so that they work together.
*/
connect: function () {
this.coordinateMoveEndSearch();
this.getConnectors().forEach((connector) => connector.connect());
},

Expand All @@ -227,6 +228,54 @@ define([
this.get("filtersMapConnector").disconnect(resetSpatialFilter);
this.get("filtersSearchConnector").disconnect();
this.get("mapSearchConnector").disconnect();
this.resetMoveEndSearch();
},

/**
* Coordinate behaviour between the two map related sub-connectors when
* the map extent changes. This is necessary to reduce the number of
* search queries. We keep the moveEnd behaviour within the sub-connectors
* so that each sub-connector still functions independently from this
* coordinating connector.
*/
coordinateMoveEndSearch: function () {
// Undo any previous coordination, if any
this.resetMoveEndSearch();

const map = this.get("map");
const mapConnectors = this.getMapConnectors();

// Stop the sub-connectors from doing anything on moveEnd by setting
// their method they call on moveEnd to null
mapConnectors.forEach((connector) => {
connector.set("onMoveEnd", null);
});

// Set the single moveEnd listener here, and run the default moveEnd
// behaviour for each sub-connector. This effectively triggers only one
// search per moveEnd.
this.listenTo(map, "moveEnd", function () {
mapConnectors.forEach((connector) => {
const moveEndFunc = connector.defaults().onMoveEnd;
if (typeof moveEndFunc === "function") {
moveEndFunc.call(connector);
}
});
});
},

/**
* Undo the coordination of the two map related sub-connectors when the
* map extent changes. Reset the moveEnd behaviour of the sub-connectors
* to their defaults.
* @see coordinateMoveEndSearch
*/
resetMoveEndSearch: function () {
this.stopListening(this.get("map"), "moveEnd");
const mapConnectors = this.getMapConnectors();
mapConnectors.forEach((connector) => {
connector.set("onMoveEnd", connector.defaults().onMoveEnd);
});
},

/**
Expand All @@ -238,7 +287,14 @@ define([
* remove any spatial constraints from the search.
*/
disconnectFiltersMap: function (resetSpatialFilter = false) {
this.get("filtersMapConnector").disconnect(resetSpatialFilter);
const [mapSearch, filtersMap] = this.getMapConnectors();

if (mapSearch.get("isConnected")) {
this.resetMoveEndSearch();
mapSearch.set("onMoveEnd", mapSearch.defaults().onMoveEnd);
}

filtersMap.disconnect(resetSpatialFilter);
},

/**
Expand All @@ -247,7 +303,12 @@ define([
* the extent of the map view.
*/
connectFiltersMap: function () {
this.get("filtersMapConnector").connect();
const [mapSearch, filtersMap] = this.getMapConnectors();

if (mapSearch.get("isConnected")) {
this.coordinateMoveEndSearch();
}
filtersMap.connect();
},

/**
Expand All @@ -265,7 +326,7 @@ define([
*/
removeSpatialFilter: function () {
this.get("filtersMapConnector").removeSpatialFilter();
}
},
}
);
});
23 changes: 20 additions & 3 deletions src/js/models/connectors/Map-Search.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,15 @@ define([
* @type {object}
* @property {SolrResults} searchResults
* @property {Map} map
* @property {function} onMoveEnd A function to call when the map is
* finished moving. This function will be called with the connector as
* 'this'.
*/
defaults: function () {
return {
searchResults: null,
map: null,
onMoveEnd: this.onMoveEnd
};
},

Expand Down Expand Up @@ -166,9 +170,10 @@ define([
// layer again and update the search results (thereby updating the
// facet counts on the GeoHash layer)
this.listenTo(map, "moveEnd", function () {
this.showGeoHashLayer();
this.updateFacet();
searchResults.trigger("reset");
const moveEndFunc = this.get("onMoveEnd");
if (typeof moveEndFunc === "function") {
moveEndFunc.call(this);
}
});

// When a new search is being performed, hide the GeoHash layer to
Expand All @@ -181,6 +186,18 @@ define([
this.set("isConnected", true);
},

/**
* Functions to perform when the map has finished moving. This is separated into its own method
* so that external models can manipulate the behavior of this function.
* See {@link MapSearchFiltersConnector#onMoveEnd}
*/
onMoveEnd: function () {
const searchResults = this.get("searchResults");
const map = this.get("map");
this.showGeoHashLayer();
this.updateFacet();
},

/**
* Make the geoHashLayer invisible.
* @fires CesiumGeohash#change:visible
Expand Down
33 changes: 29 additions & 4 deletions src/js/models/filters/SpatialFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ define([
operator: "OR",
fieldsOperator: "OR",
matchSubstring: false,
maxGeohashValues: 100,
// 1024 is the default limit in Solr for boolean clauses, limit even
// more to allow for other filters
maxGeohashValues: 900,
});
},

Expand Down Expand Up @@ -125,6 +127,20 @@ define([
};
},

/**
* Returns true if the bounds set on the filter covers the entire earth
* @returns {boolean}
*/
coversEarth: function () {
const bounds = this.getBounds();
return (
bounds.north === 90 &&
bounds.south === -90 &&
bounds.east === 180 &&
bounds.west === -180
);
},

/**
* Given the current coordinates and height set on the model, update the
* fields and values to match the geohashes that cover the area. This will
Expand All @@ -136,8 +152,17 @@ define([
updateFilterFromExtent: function () {
try {
this.validateCoordinates();
let geohashes = new Geohashes();
geohashes.addGeohashesByBounds(this.getBounds(), true, 5000, true);

// If the bounds are global there is no spatial constraint
if (this.coversEarth()) {
this.set({ fields: [], values: [] });
return;
}

const geohashes = new Geohashes();
const bounds = this.getBounds();
const limit = this.get("maxGeohashValues");
geohashes.addGeohashesByBounds(bounds, true, limit, true);
this.set({
fields: this.precisionsToFields(geohashes.getPrecisions()),
values: geohashes.getAllHashStrings(),
Expand Down Expand Up @@ -192,10 +217,10 @@ define([
return "";
}

// Merge into the minimal num. of geohashes to reduce query size
// TODO: we may still want this option for when spatial filters are
// built other than with the current view extent from the map.
// geohashes = geohashes.consolidate();

const precisions = geohashes.getPrecisions();

// Just use a regular Filter if there is only one level of geohash
Expand Down
45 changes: 8 additions & 37 deletions src/js/models/maps/assets/CesiumGeohash.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ define([
color: "#f3e227",
}),
showLabels: true,
maxGeoHashes: 1000,
maxGeoHashes: 4000,
});
},

Expand Down Expand Up @@ -151,13 +151,11 @@ define([
* @returns {number} The precision level.
*/
getPrecision: function () {
try {
const height = this.get("mapModel").get("currentViewExtent").height;
return this.get("geohashes").heightToPrecision(height);
} catch (e) {
const precisions = this.get("geohashes").getPrecisions();
return Math.min(...precisions);
}
const limit = this.get("maxGeoHashes");
const geohashes = this.get("geohashes")
const bounds = this.get("mapModel").get("currentViewExtent");
const area = geohashes.getBoundingBoxArea(bounds);
return this.get("geohashes").getMaxPrecision(area, limit);
},

/**
Expand Down Expand Up @@ -210,29 +208,6 @@ define([
if (limitToExtent) {
geohashes = this.getGeohashesForExtent();
}
const limit = this.get("maxGeoHashes");
if(typeof limit === 'number' && geohashes.length > limit) {
geohashes = this.reduceGeohashes(geohashes, limit);
}
return geohashes;
},

/**
* Reduce the number of geohashes to no more than the specified number.
* @param {Geohashes} geohashes The geohashes to reduce.
* @param {number} limitToNum The maximum number of geohashes to return.
* @returns {Geohashes} The reduced geohashes.
*/
reduceGeohashes: function (geohashes, limitToNum = 1000) {
// For now assume that we will want to summarize the combined property
// of interest by summing the values.
const propertySummaries = {};
propertySummaries[this.getPropertyOfInterest()] = function (vals) {
return vals.reduce((a, b) => a + b, 0);
}
while (geohashes.length > limitToNum) {
geohashes = geohashes.clone().reducePrecision(1, propertySummaries);
}
return geohashes;
},

Expand Down Expand Up @@ -296,12 +271,8 @@ define([
model.set("cesiumOptions", cesiumOptions);
// Create the model like a regular GeoJSON data source
CesiumVectorData.prototype.createCesiumModel.call(this, recreate);
} catch (error) {
console.log(
"There was an error creating a CesiumGeohash model" +
". Error details: ",
error
);
} catch (e) {
console.log("Error creating a CesiumGeohash model. ", e);
}
},

Expand Down
Loading

0 comments on commit be0f061

Please sign in to comment.