From 1a30979d2c830fc4c7ef0ea346200b876107f0e0 Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Tue, 30 Jun 2026 18:48:09 +0200 Subject: [PATCH 1/6] fix sort order of overlayed features on click --- app/javascript/maplibre/layers/layer.js | 32 +++++++++++++++---------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/app/javascript/maplibre/layers/layer.js b/app/javascript/maplibre/layers/layer.js index 0ede03d0..2baa3f38 100644 --- a/app/javascript/maplibre/layers/layer.js +++ b/app/javascript/maplibre/layers/layer.js @@ -191,24 +191,32 @@ export class Layer { filter: ['!', ['has', 'cluster']] }) - // Sort by ID so cycling order is stable even after frontFeature() reorders the source - const stack = [...new Map(allFeatures + // queryRenderedFeatures returns features top-most first → visual z-order + const selectable = allFeatures .filter(f => !f.properties?.cluster && SELECTABLE_SOURCE_PREFIXES.some(p => f.source.startsWith(p))) - .map(f => [f.id, f])).values()] - .sort((a, b) => String(a.id).localeCompare(String(b.id))) - if (!stack.length) { return } const isViewMode = window.gon.map_mode === 'ro' || e.originalEvent.shiftKey - const clickableStack = isViewMode - ? stack.filter(f => f.properties?.onclick !== false) - : stack + const clickable = isViewMode + ? selectable.filter(f => f.properties?.onclick !== false) + : selectable + if (!clickable.length) { return } + + // Visual top = first in z-order (matches hover behavior) + const visualTop = clickable[0] - if (!clickableStack.length) { return } + // Stable cycling order, deduped by id, independent of z-order reshuffling by frontFeature() + const clickableStack = [...new Map(clickable.map(f => [f.id, f])).values()] + .sort((a, b) => String(a.id).localeCompare(String(b.id))) const currentIdx = clickableStack.findIndex(f => f.id === highlightedFeatureId) - let feature = (currentIdx === -1 || currentIdx === clickableStack.length - 1) - ? clickableStack[0] - : clickableStack[currentIdx + 1] + let feature + if (!stickyFeatureHighlight || currentIdx === -1) { + feature = visualTop // fresh click → top feature + } else { + feature = currentIdx === clickableStack.length - 1 // re-click → advance cycle + ? clickableStack[0] + : clickableStack[currentIdx + 1] + } if (isViewMode) { From 64c97833182cb7c24ec1d8533384ebed4cc3e6fb Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Fri, 3 Jul 2026 00:54:07 +0200 Subject: [PATCH 2/6] load geojson in maplibre web worker out of main thread --- app/controllers/maps_controller.rb | 17 +++-- app/javascript/channels/map_channel.js | 7 +- app/javascript/maplibre/layers/geojson.js | 78 +++++++++++++++------ app/javascript/maplibre/layers/layers.js | 84 +++++++++++++++-------- app/javascript/maplibre/map.js | 9 ++- app/models/feature.rb | 4 +- app/models/map.rb | 14 ++-- app/views/maps/maplibre.haml | 9 --- app/views/maps/modals/_share.haml | 2 +- config/routes.rb | 1 + spec/features/map_share_spec.rb | 2 +- spec/models/map_spec.rb | 19 +++++ spec/requests/maps_controller_spec.rb | 31 +++++++++ 13 files changed, 201 insertions(+), 76 deletions(-) diff --git a/app/controllers/maps_controller.rb b/app/controllers/maps_controller.rb index 0cfb745f..b283160d 100644 --- a/app/controllers/maps_controller.rb +++ b/app/controllers/maps_controller.rb @@ -1,11 +1,11 @@ class MapsController < ApplicationController include MapListFilters - before_action :set_map, only: %i[show properties feature destroy copy] - before_action :set_map_mode, only: %i[show] + before_action :set_map, only: %i[show properties feature layer destroy copy] + before_action :set_map_mode, only: %i[show layer] before_action :join, only: %i[show] before_action :set_global_js_values, only: %i[show tutorial] - before_action :check_permissions, only: %i[show properties] + before_action :check_permissions, only: %i[show properties layer] before_action :require_login, only: %i[my create copy] before_action :require_map_owner, only: %i[destroy] @@ -52,6 +52,7 @@ def show gon.rails_env = Rails.env gon.csrf_token = form_authenticity_token gon.map_properties = @map_properties + gon.map_layers = @map.layers.map(&:to_summary_json) case params["engine"] when "deck" @@ -64,7 +65,7 @@ def show # updated_at bumps on any feature/layer/map change via the touch chain, # so it's a sufficient (and private) validator for a 304 on repeat loads. if stale?(etag: @map.updated_at) - render json: @map.to_json + render json: @map.to_json(include_features: params[:export].present?) end end format.geojson { render json: @map.to_geojson } @@ -115,6 +116,14 @@ def feature end end + def layer + layer = @map.layers.find(params["layer_id"]) + head :not_found and return unless layer + # updated_at bumps on any feature/layer change via the touch chain, + # so it's a sufficient validator for a 304 on repeat loads. + render json: layer.to_geojson if stale?(etag: layer.updated_at) + end + # Turbo sends the DELETE request automatically with Content-Type: text/vnd.turbo-stream.html # We can return a turbo stream command that removes the map element in place # To avoid turbo_stream response, force format :html diff --git a/app/javascript/channels/map_channel.js b/app/javascript/channels/map_channel.js index 4782dfbd..9a85a141 100644 --- a/app/javascript/channels/map_channel.js +++ b/app/javascript/channels/map_channel.js @@ -1,6 +1,6 @@ import consumer from 'channels/consumer' import { createLayerInstance } from 'maplibre/layers/factory' -import { initializeLayerSources, initializeLayerStyles, layers, loadLayerDefinitions } from 'maplibre/layers/layers' +import { initializeLayerSources, initializeLayerStyles, layers, loadLayerDefinitions, resetLayerInitialization } from 'maplibre/layers/layers' import { destroyFeature, initializeMaplibreProperties, map, @@ -46,9 +46,12 @@ export function initializeSocket () { // before window.gon catches up, racing against tests and any code that // reads map_properties on reconnect. if (channelStatus === 'off') { + // Rebuild layers directly (rather than initializeLayers()) to force a refetch and handle + // a possible basemap change; reset the memoization so a later initializeLayers() re-runs. + resetLayerInitialization() reloadMapProperties().then(() => { initializeMaplibreProperties() - loadLayerDefinitions().then(async () => { + loadLayerDefinitions({ refetch: true }).then(async () => { // If basemap actually changed, setBackgroundMapLayer() will trigger // initializeStyles() via style.load (which re-initializes layer sources/styles). // If not, we re-initialize them directly to catch up on any missed updates. diff --git a/app/javascript/maplibre/layers/geojson.js b/app/javascript/maplibre/layers/geojson.js index 1691c0b6..be644a66 100644 --- a/app/javascript/maplibre/layers/geojson.js +++ b/app/javascript/maplibre/layers/geojson.js @@ -1,7 +1,7 @@ import { buffer } from "@turf/buffer" import { draw, select } from 'maplibre/edit' import { initializeKmMarkerStyles, renderKmMarkers } from 'maplibre/layers/geojson/km_markers' -import { detectLevels, filterFeaturesByLevel } from 'maplibre/layers/geojson/levels' +import { detectLevels, filterFeaturesByLevel, getActiveLevel } from 'maplibre/controls/levels' import { initializeExtrasLabelStyles, renderRouteExtras } from 'maplibre/layers/geojson/route_extras' import { Layer } from 'maplibre/layers/layer' import { getFeature } from 'maplibre/layers/layers' @@ -21,6 +21,10 @@ export class GeoJSONLayer extends Layer { return `extrusion-source-${this.id}` } + get dataUrl() { + return `/m/${window.gon.map_id}/layer/${this.id}.geojson` + } + createSource() { super.createSource() addGeoJSONSource(this.kmMarkerSourceId, false) @@ -43,7 +47,7 @@ export class GeoJSONLayer extends Layer { initializeExtrasLabelStyles(this.routeExtrasSourceId) initializeViewStyles(this.extrusionSourceId) - // Exclude features with route extras from the main line layers (they're rendered in route-extras-source instead) + // Exclude features with route extras from the main linestring layers (they're rendered in route-extras-source instead) const mainLineFilter = ['all', ['==', ['geometry-type'], 'LineString'], ['!', ['has', 'show-route-extras']], @@ -58,17 +62,51 @@ export class GeoJSONLayer extends Layer { map.setLayoutProperty(`line-layer_${this.routeExtrasSourceId}`, 'line-cap', 'butt') this.setupEventHandlers() - this.render() - return Promise.resolve() + return this.loadData() + } + + // setData(url) lets MapLibre fetch AND parse the features in its web worker (off the main + // thread, so the UI stays responsive even for large layers); once loaded we read them back + // via getData() into this.layer.geojson (for lookup/sync/derived sources) without a 2nd request. + loadData() { + const sourceId = this.sourceId + const source = map.getSource(sourceId) + return new Promise((resolve) => { + const cleanup = () => { + map.off('sourcedata', onData) + map.off('error', onError) + } + const onData = (e) => { + // make sure to only act on the completed load of this source + if (e.sourceId !== sourceId || e.sourceDataType === 'metadata' || !map.isSourceLoaded(sourceId)) { return } + cleanup() + source.getData() + .then(geojson => { this.layer.geojson = geojson; this.render(true, { sourceLoaded: true }); resolve(geojson) }) + .catch(error => { console.error(`Failed to read data for ${sourceId}`, error); resolve() }) + } + // A failed URL fetch fires an 'error' event (not a completed 'sourcedata'). Resolve rather + // than reject so a single failing layer doesn't hang initializeLayerStyles' Promise.all. + const onError = (e) => { + if (e.sourceId !== sourceId) { return } + cleanup() + console.error(`Failed to load data for ${sourceId}`, e.error) + resolve() + } + map.on('sourcedata', onData) + map.on('error', onError) + source.setData(this.dataUrl) + }) } - render(resetDraw = true) { - console.log("Redraw: Setting source data for geojson layer", this.layer) + // sourceLoaded=true means the main MapLibre source already holds this.layer.geojson + // (loadData just streamed it from the layer URL). When no level filter is active, the + // source already holds exactly the features we'd upload, so skip the redundant re-parse. + render(resetDraw = true, { sourceLoaded = false } = {}) { if (!this.layer?.geojson?.features) { return } - this.ensureFeaturePropertyIds() + const source = map.getSource(this.sourceId) + if (!source) { console.warn(`Source ${this.sourceId} not found, skipping render`); return } - // Signal that GeoJSON is re-rendering (set to false) - map.getContainer().setAttribute('data-geojson-loaded', 'false') + this.ensureFeaturePropertyIds() // Detect available levels first so activeLevel is defaulted before filtering detectLevels() @@ -79,19 +117,19 @@ export class GeoJSONLayer extends Layer { renderKmMarkers(filteredFeatures, this.kmMarkerSourceId) renderRouteExtras(filteredFeatures, this.routeExtrasSourceId) this.renderExtrusionLines(filteredFeatures) - const geojson = { type: 'FeatureCollection', features: filteredFeatures } - const source = map.getSource(this.sourceId) - if (!source) { - console.warn(`Source ${this.sourceId} not found, skipping render`) - return - } - source.setData(geojson, false) - - // Wait for MapLibre to complete the render, then signal completion - map.once('render', () => { + if (sourceLoaded && !getActiveLevel()) { + // MapLibre's URL load already holds exactly this set; don't re-parse it. map.getContainer().setAttribute('data-geojson-loaded', 'true') - }) + } else { + console.log("Redraw: Setting source data for geojson layer", this.layer) + map.getContainer().setAttribute('data-geojson-loaded', 'false') + source.setData({ type: 'FeatureCollection', features: filteredFeatures }, false) + // Wait for MapLibre to complete the render, then signal completion + map.once('render', () => { + map.getContainer().setAttribute('data-geojson-loaded', 'true') + }) + } this.resetDrawFeatures(resetDraw) } diff --git a/app/javascript/maplibre/layers/layers.js b/app/javascript/maplibre/layers/layers.js index 8b61ac50..516f2381 100644 --- a/app/javascript/maplibre/layers/layers.js +++ b/app/javascript/maplibre/layers/layers.js @@ -1,13 +1,28 @@ import * as functions from 'helpers/functions' import { resetLevels } from 'maplibre/controls/levels' import { createLayerInstance } from 'maplibre/layers/factory' -import { map, sortLayers } from 'maplibre/map' +import { sortLayers } from 'maplibre/map' export let layers // Layer instances: GeoJSONLayer, OverpassLayer, WikipediaLayer, BasemapLayer // Cached promise to ensure initializeLayers only runs once let initializePromise = null +// Layer setup workflow (three steps; initializeLayers() runs all three and is memoized): +// +// initializeLayers() <- use this for normal setup +// ├─> loadLayerDefinitions() build Layer instances from summaries (from gon on initial +// │ load, or refetched from /m/:id.json). No sources/features yet. +// ├─> initializeLayerSources() create the MapLibre source for every layer +// └─> initializeLayerStyles() for visible layers: apply styles + layer.initialize() +// └─> loadData() per layer type: GeoJSON streams its features +// from the layer URL (MapLibre setData, read back via +// getData); Overpass/Wikipedia query their APIs; Raster tiles +// +// Reconnect (map_channel.js) assembles these steps directly instead of calling +// initializeLayers(), to bypass memoization and force a refetch: +// loadLayerDefinitions({ refetch: true }) -> initializeLayerSources() -> initializeLayerStyles(). + /** * Resets the initialization state when navigating to a new map. * This allows layers to be re-initialized from scratch. @@ -21,6 +36,12 @@ export function resetInitializationState() { layers = null } +// Clear the initializeLayers() memoization without tearing down existing layers, so a later +// initializeLayers() re-runs from scratch. Used on reconnect, which rebuilds layers directly. +export function resetLayerInitialization() { + initializePromise = null +} + /** * Loads layer definitions from server and initializes them. * Combines loadLayerDefinitions(), initializeLayerSources(), and initializeLayerStyles() @@ -49,35 +70,34 @@ export async function initializeLayers() { } /** - * Loads layer definitions from server. - * Prefer using initializeLayers() for full initialization. + * Loads layer definitions (summaries only; feature geometry loads per-layer from the + * layer URL via GeoJSONLayer.loadData). On initial load these are embedded in gon, so no + * request is needed. On reconnect, pass { refetch: true } to pull fresh state from the server. */ -export function loadLayerDefinitions() { +export function loadLayerDefinitions({ refetch = false } = {}) { layers = null - const host = new URL(window.location.href).origin - const url = host + '/m/' + window.gon.map_id + '.json' - - // Reuse the in-flight fetch kicked off by the inline script in maplibre.haml, - // if it matches this map. Falls back to a fresh fetch otherwise (Turbo navigation, - // missing script, etc). - const cached = window._mapJsonForId - window._mapJsonForId = null - const dataPromise = (cached && cached.id === window.gon.map_id) - ? cached.promise - : fetch(url).then(response => { - if (!response.ok) { throw new Error('Network response was: ', response) } - return response.json() - }) - - return dataPromise - .then(data => { - console.log('Loaded map layer definitions from server: ', data.layers) - // make sure we're still showing the map the request came from - if (window.gon.map_properties.public_id !== data.properties.public_id) { return } - layers = data.layers.map(l => createLayerInstance(l)) - window._layers = layers - map.fire('layers.load', { detail: { message: `Map data (${layers.length} layers) loaded from server` } }) + + const createLayers = (data) => { + console.log('Loaded map layer definitions: ', data.layers) + // make sure we're still showing the map the definitions came from + if (window.gon.map_properties.public_id !== data.properties.public_id) { return } + layers = data.layers.map(l => createLayerInstance(l)) + window._layers = layers + // console.log(`Map layers (${layers.length}) instantiated`) + } + + if (!refetch && window.gon.map_layers) { + createLayers({ properties: window.gon.map_properties, layers: window.gon.map_layers }) + return Promise.resolve() + } + + const url = '/m/' + window.gon.map_id + '.json' + return fetch(url) + .then(response => { + if (!response.ok) { throw new Error('Network response was: ' + response.status) } + return response.json() }) + .then(createLayers) .catch(error => { console.error('Failed to fetch map layers:', error) throw error @@ -124,8 +144,16 @@ export async function initializeLayerStyles(id = null) { const promises = initLayers.map(layer => layer.initialize()) + // Hidden geojson layers aren't styled, but their feature data is still loaded so feature + // lookups (deep links, onclick targets, undo) and layer feature counts work, matching the + // pre-streaming behaviour when features were embedded in the map JSON. Overpass/Wikipedia + // stay lazy because their loadData() hits external APIs. + let dataOnlyLayers = layers.filter(l => l.show === false && l.type === 'geojson') + if (id) { dataOnlyLayers = dataOnlyLayers.filter(l => l.id === id) } + promises.push(...dataOnlyLayers.map(layer => layer.loadData())) + await Promise.all(promises).then(_results => { - map.fire('geojson.load', { detail: { message: 'geojson source + styles loaded' } }) + console.log('geojson source + styles loaded') // re-sort layers after style changes sortLayers() functions.e('#layer-loading', e => { e.classList.add('hidden') }) diff --git a/app/javascript/maplibre/map.js b/app/javascript/maplibre/map.js index 8a235160..bfc402ad 100644 --- a/app/javascript/maplibre/map.js +++ b/app/javascript/maplibre/map.js @@ -253,7 +253,7 @@ export function addGeoJSONSource(sourceName, cluster=false) { // https://maplibre.org/maplibre-style-spec/sources/#geojson // console.log("Adding source: " + sourceName) if (map.getSource(sourceName)) { - console.log('Source ' + sourceName + ' already exists, skipping add') + console.log('Source ' + sourceName + ' already exists, re-using it') return } map.addSource(sourceName, { @@ -457,10 +457,13 @@ export function upsert (updatedFeature) { const feature = getFeature(updatedFeature.id) if (!feature) { addFeature(updatedFeature); return } - // only update feature if it was changed, disregard properties.id + // only update feature if it was changed, disregarding properties.id on both sides + // (the server now includes it for MapLibre's promoteId, so it isn't a meaningful diff) const existingFeature = JSON.parse(JSON.stringify(feature)) + const incomingFeature = JSON.parse(JSON.stringify(updatedFeature)) delete existingFeature.properties.id - if (!equal(existingFeature, updatedFeature)) { + delete incomingFeature.properties.id + if (!equal(existingFeature, incomingFeature)) { updateFeature(feature, updatedFeature) } } diff --git a/app/models/feature.rb b/app/models/feature.rb index 019f4374..7b2d19e6 100644 --- a/app/models/feature.rb +++ b/app/models/feature.rb @@ -31,10 +31,12 @@ class Feature validate :require_coords def geojson + # MapLibre's promoteId:'id' reads the id from properties, so expose it there too, + # alongside the top-level id used by feature lookups/frontFeature. { id: _id.to_s, type:, geometry:, - properties: properties || {} } + properties: (properties || {}).merge("id" => _id.to_s) } end def to_geojson diff --git a/app/models/map.rb b/app/models/map.rb index 3967677b..e6607850 100644 --- a/app/models/map.rb +++ b/app/models/map.rb @@ -162,15 +162,15 @@ def self.provider_keys indoorequal: ENV["INDOOREQUAL_KEY"] } end - def to_json - all_layers = layers.to_a - all_features = Feature.in(layer: all_layers.map(&:id)).group_by(&:layer_id) - { properties: properties, layers: all_layers.map { |l| + # By default returns layer summaries only (features stream per-layer from the layer URL). + # Pass include_features: true for a self-contained, re-importable export (Share > Map export). + def to_json(include_features: false) + layers_json = layers.map do |l| json = l.to_summary_json - json[:geojson] = { type: "FeatureCollection", - features: l.order_features(all_features[l.id] || []).map(&:geojson) } + json[:geojson] = l.to_geojson if include_features json - } }.to_json + end + { properties: properties, layers: layers_json }.to_json end # flattened geojson collection of all layers diff --git a/app/views/maps/maplibre.haml b/app/views/maps/maplibre.haml index 77ba4971..01ba49cc 100644 --- a/app/views/maps/maplibre.haml +++ b/app/views/maps/maplibre.haml @@ -6,15 +6,6 @@ - content_for :title, "Mapforge map: #{(@map.name.presence || 'Unnamed map')}" - content_for :head do - - # Kick off the layer-data fetch as early as possible (in parallel with maplibre-gl import - - # and the rest of head parsing). loadLayerDefinitions() consumes this in-flight promise - - # instead of issuing a second request. - %script{ nonce: content_security_policy_nonce } - :plain - window._mapJsonForId = (function() { - var id = #{params[:id].to_json}; - return { id: id, promise: fetch('/m/' + id + '.json').then(function(r) { return r.json(); }) }; - })(); - # Speed up default basemap tile fetches (versatilesColorful uses tiles.versatiles.org) %link{ rel: "preconnect", href: "https://tiles.versatiles.org", crossorigin: true } %link{ rel: "dns-prefetch", href: "https://tiles.versatiles.org" } diff --git a/app/views/maps/modals/_share.haml b/app/views/maps/modals/_share.haml index 09557c40..577d574d 100644 --- a/app/views/maps/modals/_share.haml +++ b/app/views/maps/modals/_share.haml @@ -79,7 +79,7 @@ %i.bi.bi-download GPX %button.btn.btn-secondary.btn-download{ type: "button" } - =link_to map_json_path(@map.public_id),target: '_blank' do + =link_to map_json_path(@map.public_id, export: true), target: '_blank' do %i.bi.bi-download Map %span.d-none.d-sm-inline export diff --git a/config/routes.rb b/config/routes.rb index 14cb3343..cd97437b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -22,6 +22,7 @@ get "/:id.gpx" => "maps#show", :as => :map_gpx, :constraints => { id: ID_PATTERN }, :defaults => { format: "gpx" } get "/:id/properties" => "maps#properties", :as => :map_properties, :constraints => { id: ID_PATTERN } get "/:id(/:name)" => "maps#show", :as => :map, :format => :html, :constraints => { id: ID_PATTERN, name: NAME_PATTERN } + get "/:id/layer/:layer_id.geojson" => "maps#layer", :as => :map_layer_geo, :constraints => { id: ID_PATTERN, layer_id: ID_PATTERN }, :defaults => { format: "geojson" } get "/:id/feature/:feature_id.geojson(/:name)" => "maps#feature", :as => :map_feature_geo, :constraints => { id: ID_PATTERN, feature_id: ID_PATTERN, name: NAME_PATTERN }, :defaults => { format: "geojson" } get "/:id/feature/:feature_id.gpx(/:name)" => "maps#feature", :as => :map_feature_gpx, :constraints => { id: ID_PATTERN, feature_id: ID_PATTERN, name: NAME_PATTERN }, :defaults => { format: "gpx" } diff --git a/spec/features/map_share_spec.rb b/spec/features/map_share_spec.rb index f48f1953..200d62a3 100644 --- a/spec/features/map_share_spec.rb +++ b/spec/features/map_share_spec.rb @@ -31,7 +31,7 @@ end it "has share map export link" do - expect(page).to have_link("Map export", href: "/m/" + subject.public_id + ".json") + expect(page).to have_link("Map export", href: "/m/" + subject.public_id + ".json?export=true") end end diff --git a/spec/models/map_spec.rb b/spec/models/map_spec.rb index a3667187..2cbb0a7c 100644 --- a/spec/models/map_spec.rb +++ b/spec/models/map_spec.rb @@ -7,6 +7,25 @@ it "included properties & layers" do expect(map.to_json).to be_a(String) end + + it "returns layer summaries without embedded geojson" do + parsed = JSON.parse(map.to_json) + expect(parsed).to have_key("properties") + expect(parsed["layers"]).to be_present + parsed["layers"].each do |layer| + expect(layer).to have_key("id") + expect(layer).not_to have_key("geojson") + end + end + + it "embeds layer geojson when include_features is true (re-importable export)" do + parsed = JSON.parse(map.to_json(include_features: true)) + expect(parsed["layers"]).to be_present + layer_with_features = parsed["layers"].find { |l| l.dig("geojson", "features").present? } + expect(layer_with_features).to be_present + # MapLibre's promoteId:'id' reads the id from properties + expect(layer_with_features["geojson"]["features"].map { |f| f["properties"]["id"] }).to all(be_present) + end end describe "#features_count" do diff --git a/spec/requests/maps_controller_spec.rb b/spec/requests/maps_controller_spec.rb index 37b4390b..53745f75 100644 --- a/spec/requests/maps_controller_spec.rb +++ b/spec/requests/maps_controller_spec.rb @@ -28,6 +28,37 @@ end end + describe "#layer" do + let(:layer) { map.layers.first } + + before do + create(:feature, :point, layer: layer) + create(:feature, :line_string, layer: layer) + end + + it "returns the layer's features as a GeoJSON FeatureCollection" do + get map_layer_geo_path(id: map.public_id, layer_id: layer.id) + + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + expect(body["type"]).to eq "FeatureCollection" + expect(body["features"].size).to eq 2 + # MapLibre's promoteId:'id' reads the id from properties + expect(body["features"].map { |f| f["properties"]["id"] }).to all(be_present) + end + + it "returns 404 for an unknown layer id" do + get map_layer_geo_path(id: map.public_id, layer_id: BSON::ObjectId.new.to_s) + expect(response).to have_http_status(:not_found) + end + + it "denies access to a private map for non-owners" do + map.update!(view_permission: "private") + get map_layer_geo_path(id: map.public_id, layer_id: layer.id) + expect(response).to redirect_to(maps_path) + end + end + describe "#map" do before do allow_any_instance_of(ApplicationController).to receive(:session).and_return({ user_id: user.id }) From 46045181a99ad63560600d2a4159c864a7810cd4 Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Fri, 3 Jul 2026 18:36:08 +0200 Subject: [PATCH 3/6] catch nil layer --- app/javascript/maplibre/layers/layers.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/javascript/maplibre/layers/layers.js b/app/javascript/maplibre/layers/layers.js index 3641da57..f9e94ad2 100644 --- a/app/javascript/maplibre/layers/layers.js +++ b/app/javascript/maplibre/layers/layers.js @@ -107,7 +107,11 @@ export function loadLayerDefinitions({ refetch = false } = {}) { }) .then(data => { createLayers(data) - map.fire('layers.load', { detail: { message: `Map data (${layers.length} layers) loaded from server` } }) + // createLayers bails (leaving layers null) if we've since navigated to another map; + // only announce the load when it actually happened. + if (layers) { + map.fire('layers.load', { detail: { message: `Map data (${layers.length} layers) loaded from server` } }) + } }) .catch(error => { console.error('Failed to fetch map layers:', error) From 32906b073ed129fe1edf21182f2aaa1e029f523e Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Sat, 4 Jul 2026 14:02:43 +0200 Subject: [PATCH 4/6] improve comments --- app/controllers/maps_controller.rb | 5 +---- app/javascript/channels/map_channel.js | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/app/controllers/maps_controller.rb b/app/controllers/maps_controller.rb index 7fddff53..fe0ceba5 100644 --- a/app/controllers/maps_controller.rb +++ b/app/controllers/maps_controller.rb @@ -53,8 +53,6 @@ def show gon.csrf_token = form_authenticity_token gon.map_properties = @map_properties gon.map_layers = @map.layers.map(&:to_summary_json) - # Seed the loaded-map version so the channel reconnect handler can compare it against - # /properties and skip a full reload when nothing changed (see map_channel.js). gon.map_updated_at = @map.updated_at case params["engine"] @@ -122,8 +120,7 @@ def feature def layer layer = @map.layers.find(params["layer_id"]) head :not_found and return unless layer - # updated_at bumps on any feature/layer change via the touch chain, - # so it's a sufficient validator for a 304 on repeat loads. + # Sending 304 on repeat loads un unchanged layer. render json: layer.to_geojson if stale?(etag: layer.updated_at) end diff --git a/app/javascript/channels/map_channel.js b/app/javascript/channels/map_channel.js index c7a0447c..0b48cba0 100644 --- a/app/javascript/channels/map_channel.js +++ b/app/javascript/channels/map_channel.js @@ -45,10 +45,7 @@ export function initializeSocket () { console.log('Connected to map_channel ' + window.gon.map_id) mapChannel = this window.mapChannel = mapChannel - // On reconnect (channelStatus === 'off'), defer the 'online' event until - // after the reload chain finishes — otherwise data-online='true' fires - // before window.gon catches up, racing against tests and any code that - // reads map_properties on reconnect. + // On reconnect (channelStatus === 'off'), defer the 'online' event if (channelStatus === 'off') { // Rebuild layers directly (rather than initializeLayers()) to force a refetch and handle // a possible basemap change; reset the memoization so a later initializeLayers() re-runs. From 165eded36c3917b8d53f444a1ce6a2ff64f5f81b Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Sat, 4 Jul 2026 14:08:01 +0200 Subject: [PATCH 5/6] avoid deep clone to save memory --- app/javascript/channels/map_channel.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/javascript/channels/map_channel.js b/app/javascript/channels/map_channel.js index 0b48cba0..e2b39589 100644 --- a/app/javascript/channels/map_channel.js +++ b/app/javascript/channels/map_channel.js @@ -195,13 +195,16 @@ export function initializeSocket () { }, send_message (event, data) { - // copy feature to avoid mutation - const payload = JSON.parse(JSON.stringify(data)) + // shallow copy to avoid mutating caller's data; geometry is never touched so it's left shared + const payload = { ...data } payload.map_id = window.gon.map_id payload.user_id = window.gon.user_id payload.uuid = connectionUUID // dropping properties.id before sending to server - if (payload.properties && payload.properties.id) { delete payload.properties.id } + if (payload.properties && payload.properties.id) { + payload.properties = { ...payload.properties } + delete payload.properties.id + } if (event !== 'mouse') console.log('Sending: [' + event + '] :', payload) // Call the original perform method this.perform(event, payload) From c9ccdbe69409fba7841a37a78a390ae06764e4c5 Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Sat, 4 Jul 2026 14:13:43 +0200 Subject: [PATCH 6/6] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 320827a1..70dacd05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to this project will be documented in this file. +## 2026-07 + +* Performance improvements for large maps + ## 2026-06 * Providing the official [OpenSUSE Conference 2026 map](https://mapforge.org/m/osc26/openSUSE%20Conference%202026)