openvibers/openvibe.tours Commits

Tours: place photos reach Media (and the launch's places get theirs); a city's first visit answers in 20 s (#107)

OP OpenVibers committed 5 days ago · 0c731dcf03 · 9 files, +187 −14

* Tours: place photos reach Media, and a city's first visit answers in 20 s

- The photo cache asked Commons for 640px thumbnails, a width Commons does not render
  (400), and returned null without a word: no place photo reached Media on launch. It
  asks for 960 (a standard width), falls back to the exact imageinfo URL, and logs a
  refusal or a Media answer without a public URL.
- A city's first visit waited for the whole discovery (Seattle: over a minute, a 503
  from the proxy). It waits at most 20 s; otherwise a page says the city is being
  gathered and refreshes itself, or the city shows what is stored and refreshes.

* Places job: a photo pass for places stored without a photo

Up to 60 places per run with a Wikidata item and no photo, best ranked first, go through
the same photo lookup; a found photo is stored with its credit, and a place with none is
marked tried (migration 0010, photo_tried_at) and asked again after a week. It repairs
the places stored on launch day, when every Media copy was refused.

* Places job version: the first boot runs the photo pass at once

Changes

ddiff --git a/migrations/0010_place_photo_tries.sql b/migrations/0010_place_photo_tries.sql
nnew file mode 100644
iindex 0000000..b936c32
---- /dev/null
++++ b/migrations/0010_place_photo_tries.sql
@@@ -0,0 +1,4 @@
++-- phase: expand
++-- When the places job last tried to find a photo for a place that has none, so a place with no free photo is not
++-- asked about on every run (the photo pass retries after a week).
++ALTER TABLE tours_places ADD COLUMN IF NOT EXISTS photo_tried_at timestamptz;
ddiff --git a/server/http/pages.js b/server/http/pages.js
iindex d0b39e1..0fad793 100644
---- a/server/http/pages.js
++++ b/server/http/pages.js
@@@ -86,21 +86,37 @@ function createPageRoutes(ctx) {
});
});
++ // A city's first visit gathers it from Nominatim, Wikidata, Wikivoyage (and its districts) and OpenStreetMap, with
++ // photos: more than a minute for a big city, past the proxy's timeout (Seattle answered 503, 11-08). The page waits
++ // at most CITY_WAIT_MS; the work goes on, and the page answers with what is there and refreshes itself.
++ const CITY_WAIT_MS = 20_000;
++ const soon = (promise, label) => {
++ const work = Promise.resolve(promise).catch((err) => { ctx.log.warn(`[Tours ${label}] ${err.message}`); return null; });
++ return Promise.race([work.then((value) => ({ done: true, value })), new Promise((ok) => setTimeout(() => ok({ done: false }), CITY_WAIT_MS).unref())]);
++ };
r.get('/city/:slug', async (req, res) => {
let city = await store.cityBySlug(String(req.params.slug || ''));
++ let gathering = false;
if (!city) {
-- try { city = await discover.ensureCity(String(req.params.slug || '').replace(/-/g, ' ')); }
-- catch (err) { ctx.log.warn(`[Tours city lookup] ${err.message}`); }
-- }
-- else if (city.place_count < 24) {
-- try { city = await discover.ensureCity(city.name); }
-- catch (err) { ctx.log.warn(`[Tours city refresh] ${err.message}`); }
++ const name = String(req.params.slug || '').replace(/-/g, ' ');
++ const got = await soon(discover.ensureCity(name), 'city lookup');
++ if (!got.done) {
++ res.set('Refresh', '12');
++ return page(req, res, { index: false, title: `Gathering ${name}`, body: raw(views.gathering(name)) }, 200);
++ }
++ city = got.value;
++ } else if (city.place_count < 24) {
++ // More places are on their way; this visit shows what is stored now.
++ const got = await soon(discover.ensureCity(city.name), 'city refresh');
++ if (got.done && got.value) city = got.value; else gathering = !got.done;
}
if (!city) return page(req, res, { index: false, title: 'Not found', body: raw(views.notFound()) }, 404);
if (city.slug !== req.params.slug) return res.redirect(301, `/city/${encodeURIComponent(city.slug)}`);
if (!city.osm_fetched_at && (!city.osm_attempted_at || Date.now() - new Date(city.osm_attempted_at).getTime() > 15 * 60_000)) {
-- try { await jobs.places.populateCity(city); } catch (err) { ctx.log.warn(`[Tours city places] ${err.message}`); }
++ const got = await soon(jobs.places.populateCity(city), 'city places');
++ if (!got.done) gathering = true;
}
++ if (gathering) res.set('Refresh', '15');
const [places, routes, live, stays] = await Promise.all([
store.cityPlaces(city.id, { limit: 60 }), store.cityRoutes(city.id), store.cityLive(city.id), store.cityStays(city.id),
]);
ddiff --git a/server/jobs/places.js b/server/jobs/places.js
iindex 4264c72..4550bbd 100644
---- a/server/jobs/places.js
++++ b/server/jobs/places.js
@@@ -21,7 +21,7 @@ const { isPlace } = require('../places/is-place');
// A run under older rules is not fresh (jobs/index.js): name-en-1 stored OpenStreetMap's English names; wv-text-1
// rewrites the stored Wikivoyage places' text with the current parser.
--const PLACES_VERSION = 'notable-places-2';
++const PLACES_VERSION = 'notable-places-3-photo-pass'; // 11-08: the first boot runs the photo pass at once
function commonsFile(value) {
const raw = String(value || '').trim();
@@@ -243,6 +243,19 @@ function createPlacesJob({ config, s, store, fetchImpl, log = console }) {
}
}
++ // 1b. Places stored without a photo (a copy refused on 11-08, or a photo added to Commons since): a bounded
++ // batch per run, tried again after a week.
++ try {
++ const bare = await store.placesWithoutPhoto({ limit: 60 });
++ if (bare.length) {
++ const pictured = await photosFor(bare.map((r) => ({ id: r.id, wikidataId: r.wikidata_id, name: r.name })));
++ result.photos = 0;
++ for (const p of pictured) if (await store.setPlacePhoto(p.id, p)) result.photos += 1;
++ }
++ } catch (err) {
++ result.errors.push(`photos: ${err && err.message}`);
++ }
++
// 2. OpenStreetMap for the cities whose OSM data is oldest, bounded per run.
const want = Math.min(100, Math.max(0, Number(osmLimit) || 0));
if (!want) return result;
ddiff --git a/server/places/photo-cache.js b/server/places/photo-cache.js
iindex 5cc21c2..a8ed1c2 100644
---- a/server/places/photo-cache.js
++++ b/server/places/photo-cache.js
@@@ -2,8 +2,8 @@
// A checked Commons image is copied once into Media as a public object. The file's Commons page,
// author and licence stay on the place row; Media serves only the smaller image bytes.
--function createPhotoCache({ config, s, fetchImpl = globalThis.fetch, log = console }) {
-- let objects;
++function createPhotoCache({ config, s, fetchImpl = globalThis.fetch, log = console, objectsClient = null }) {
++ let objects = objectsClient;
function client() {
if (!objects) {
const { createServiceTokenClient } = require('openvibe-sdk/auth');
@@@ -27,8 +27,18 @@ function createPhotoCache({ config, s, fetchImpl = globalThis.fetch, log = conso
const commons = /^(?:thumb|upload)\.wikimedia\.org$/.test(src.hostname);
const openverse = src.hostname === 'api.openverse.org' && /^\/v1\/images\/[0-9a-f-]{36}\/thumb\/$/i.test(src.pathname);
if (src.protocol !== 'https:' || (!commons && !openverse)) return null;
-- if (commons) src.pathname = src.pathname.replace(/\/\d+px-([^/]+)$/, '/640px-$1');
-- const res = await fetchImpl(src.toString(), { headers: { 'User-Agent': config.openData.userAgent }, signal: AbortSignal.timeout(12_000) });
++ // Commons renders only its standard thumbnail widths (250, 330, 500, 960, 1280 …): 640 answered 400 for every
++ // file, so no photo reached Media (11-08). Ask for 960; if that is refused (a small original cannot be enlarged),
++ // take the exact URL Commons' imageinfo returned.
++ const tries = [];
++ if (commons && /\/\d+px-[^/]+$/.test(src.pathname)) { const t = new URL(src); t.pathname = t.pathname.replace(/\/\d+px-([^/]+)$/, '/960px-$1'); tries.push(t.toString()); }
++ tries.push(src.toString());
++ let res = null;
++ for (const u of tries) {
++ res = await fetchImpl(u, { headers: { 'User-Agent': config.openData.userAgent }, signal: AbortSignal.timeout(12_000) });
++ if (res.ok) break;
++ }
++ if (!res.ok) { log.warn(`[Tours photos] ${src.hostname} answered ${res.status} for ${image.sourceUrl}`); return null; }
const type = String(res.headers.get('content-type') || '').split(';')[0].toLowerCase();
const length = Number(res.headers.get('content-length') || 0);
if (!res.ok || !/^image\/(?:jpeg|png|webp)$/.test(type) || length > 4_000_000) return null;
@@@ -37,7 +47,7 @@ function createPhotoCache({ config, s, fetchImpl = globalThis.fetch, log = conso
const file = image.sourceUrl.split('/').pop().slice(0, 100) || 'place-photo.jpg';
const obj = await client().upload(bytes, { kind: 'file', visibility: 'public', mimeType: type, filename: file });
const url = obj.public_url || obj.url;
-- if (!url || !/^https:\/\/openvibe\.media\//.test(url)) return null;
++ if (!url || !/^https:\/\/openvibe\.media\//.test(url)) { log.warn(`[Tours photos] Media answered no public openvibe.media URL for ${image.sourceUrl}`); return null; }
await s.db.exec(`INSERT INTO tours_photo_assets(source_url, media_id, media_url) VALUES ($1,$2,$3)
ON CONFLICT(source_url) DO NOTHING`, [image.sourceUrl, obj.id, url]);
return { ...image, url };
ddiff --git a/server/store.js b/server/store.js
iindex b793c45..5db8b8f 100644
---- a/server/store.js
++++ b/server/store.js
@@@ -685,7 +685,25 @@ function createStore(s, { config = {}, sharing = null } = {}) {
return { ok: true };
}
++ /** Places with a Wikidata item and no photo, not tried in the last week: the best ranked first. */
++ async function placesWithoutPhoto({ limit = 60 } = {}) {
++ return db.many(`SELECT id, wikidata_id, name FROM tours_places
++ WHERE image_url IS NULL AND wikidata_id IS NOT NULL
++ AND (photo_tried_at IS NULL OR photo_tried_at < now() - interval '7 days')
++ ORDER BY rank_score DESC NULLS LAST, id LIMIT $1`, [Math.max(1, Math.min(200, Number(limit) || 60))]);
++ }
++ /** A photo found for a stored place (credited), or only the time it was looked for. */
++ async function setPlacePhoto(id, p) {
++ if (!p || !p.imageUrl) { await db.exec('UPDATE tours_places SET photo_tried_at = now() WHERE id = $1', [id]); return false; }
++ await db.exec(`UPDATE tours_places SET image_url = $2, image_credit = $3, image_license = $4, image_license_url = $5,
++ image_source_url = $6, gallery = $7::jsonb, photo_tried_at = now(), updated_at = now() WHERE id = $1`,
++ [id, p.imageUrl, p.imageCredit ? clean(p.imageCredit, 250) : null, p.imageLicense ? clean(p.imageLicense, 120) : null,
++ p.imageLicenseUrl || null, p.imageSourceUrl || null, JSON.stringify(Array.isArray(p.gallery) ? p.gallery : [])]);
++ return true;
++ }
++
return {
++ placesWithoutPhoto, setPlacePhoto,
counts, listCities, cityBySlug, cityById, cityPlaces, cityRoutes, cityLive, placeLive, cityStays, citiesForOsm, markCityOsmFetched, markCityOsmAttempted,
placeById, updatePlaceGuide, routeById, routes, routesForPlace, placeOfTheDay, liveNow, recentTours, stays,
latestJobRuns, recordJobStart, recordJobFinish,
ddiff --git a/server/views/index.js b/server/views/index.js
iindex 5905e44..cc37c5d 100644
---- a/server/views/index.js
++++ b/server/views/index.js
@@@ -624,8 +624,15 @@ function developersPage({ jobs = [] } = {}) {
</div>`;
}
++/** A city still being gathered on its first visit (the page refreshes itself; Refresh header). */
++function gathering(name) {
++ return `<div class="tr-wrap tr-page-pad tr-narrow" id="tr-gathering"><p class="tr-kicker">First visit</p><h1>Gathering ${esc(name)}</h1>
++<p class="tr-lede">Its places are coming in from Wikivoyage, Wikidata and OpenStreetMap, with their photos and credits. This page reloads itself in a few seconds; the next visitor gets it at once.</p>
++<p><a class="tr-btn tr-btn-quiet" href="/cities">Other cities</a></p></div>`;
++}
++
function notFound() {
return `<div class="tr-wrap tr-narrow tr-page-pad" id="tr-not-found"><p class="tr-kicker">404</p><h1>Not here</h1><p class="tr-lede">That city, place, route or trip does not exist, or it is a private trip.</p><p><a class="tr-btn" href="/">Go home</a> <a class="tr-btn tr-btn-quiet" href="/cities">Browse cities</a></p></div>`;
}
--module.exports = { placeName, localName, kindUnlessNamed, HOME_FAQ, safeImage, home, citiesPage, cityPage, placePage, routePage, livePage, staysPage, flightsPage, signIn, tripNew, tripsPage, tripPage, tripGate, developersPage, notFound, placeOfDayCard, heroMotif, miniMap, SOURCE };
++module.exports = { placeName, localName, kindUnlessNamed, HOME_FAQ, safeImage, home, citiesPage, cityPage, placePage, routePage, livePage, staysPage, flightsPage, signIn, tripNew, tripsPage, tripPage, tripGate, developersPage, gathering, notFound, placeOfDayCard, heroMotif, miniMap, SOURCE };
ddiff --git a/test/city-first-visit.test.js b/test/city-first-visit.test.js
nnew file mode 100644
iindex 0000000..a43f993
---- /dev/null
++++ b/test/city-first-visit.test.js
@@@ -0,0 +1,23 @@
++'use strict';
++/**
++ * A city's first visit answers within CITY_WAIT_MS while its discovery goes on (Seattle took over a minute and the
++ * proxy answered 503, 11-08): a page that says it is gathering the city and refreshes itself, or the city with what
++ * is stored so far, also refreshing.
++ *
++ * node test/city-first-visit.test.js
++ */
++const assert = require('assert');
++const fs = require('fs');
++const path = require('path');
++const pages = fs.readFileSync(path.join(__dirname, '..', 'server', 'http', 'pages.js'), 'utf8');
++const route = pages.slice(pages.indexOf('const CITY_WAIT_MS'), pages.indexOf("store.cityPlaces(city.id, { limit: 60 })"));
++assert.match(route, /const CITY_WAIT_MS = 20_000;/, 'twenty seconds at most');
++assert.match(route, /soon\(discover\.ensureCity\(name\), 'city lookup'\)/, 'a new city is looked up with the limit');
++assert.match(route, /res\.set\('Refresh', '12'\);\s*return page\(req, res, \{ index: false, title: `Gathering \$\{name\}`, body: raw\(views\.gathering\(name\)\) \}, 200\);/, 'not yet found: the gathering page, refreshing');
++assert.match(route, /soon\(jobs\.places\.populateCity\(city\), 'city places'\)/, 'places are gathered with the limit');
++assert.match(route, /if \(gathering\) res\.set\('Refresh', '15'\);/, 'a city still filling refreshes itself');
++const views = require('../server/views');
++const html = views.gathering('Seattle <x>');
++assert.ok(html.includes('Gathering Seattle &lt;x&gt;'), 'the name is escaped');
++assert.ok(!/spinner|loading-ring/i.test(html), 'words, not a spinner');
++console.log('city first visit: answers within 20 s and refreshes itself while the city is gathered');
ddiff --git a/test/photo-cache-widths.test.js b/test/photo-cache-widths.test.js
nnew file mode 100644
iindex 0000000..d3c9344
---- /dev/null
++++ b/test/photo-cache-widths.test.js
@@@ -0,0 +1,42 @@
++'use strict';
++/**
++ * Tours' photo cache asks Commons for a thumbnail width Commons renders (960), falls back to the exact URL imageinfo
++ * returned when that is refused, and says so in the log when nothing can be copied. On 11-08 every place photo failed
++ * silently: the cache asked for 640px, which Commons answers with 400.
++ *
++ * node test/photo-cache-widths.test.js
++ */
++const assert = require('assert');
++const { createPhotoCache } = require('../server/places/photo-cache');
++
++const config = { oauth: { clientSecret: 'x' }, openData: { userAgent: 'test' }, drill: false };
++const db = { maybe: async () => null, exec: async () => {} };
++const image = { url: 'https://upload.wikimedia.org/wikipedia/commons/thumb/a/ab/Kubota.jpg/1280px-Kubota.jpg', sourceUrl: 'https://commons.wikimedia.org/wiki/File:Kubota.jpg' };
++const ok = (type = 'image/jpeg') => ({ ok: true, status: 200, headers: { get: (h) => (h === 'content-type' ? type : '1000') }, arrayBuffer: async () => new Uint8Array(1000).buffer });
++const refused = { ok: false, status: 400, headers: { get: () => 'text/html' }, arrayBuffer: async () => new ArrayBuffer(0) };
++const uploaded = [];
++const objectsClient = { upload: async (bytes) => { uploaded.push(bytes.length); return { id: 'obj_1', public_url: 'https://openvibe.media/api/v2/tours/objects/obj_1' }; } };
++
++(async () => {
++ // 1. 960px is asked first and copied.
++ let asked = [];
++ let cache = createPhotoCache({ config, s: { db }, objectsClient, log: { warn() {} }, fetchImpl: async (u) => { asked.push(u); return ok(); } });
++ let out = await cache.cache(image);
++ assert.ok(/\/960px-Kubota\.jpg$/.test(asked[0]), 'a standard Commons width first');
++ assert.strictEqual(out.url, 'https://openvibe.media/api/v2/tours/objects/obj_1');
++
++ // 2. 960 refused (a small original): the exact imageinfo URL is used.
++ asked = [];
++ cache = createPhotoCache({ config, s: { db }, objectsClient, log: { warn() {} }, fetchImpl: async (u) => { asked.push(u); return /960px/.test(u) ? refused : ok(); } });
++ out = await cache.cache(image);
++ assert.deepStrictEqual(asked.map((u) => u.replace(/.*\//, '')), ['960px-Kubota.jpg', '1280px-Kubota.jpg']);
++ assert.ok(out && out.url.startsWith('https://openvibe.media/'), 'copied from the original URL');
++
++ // 3. Both refused: null, and the log says why.
++ const warned = [];
++ cache = createPhotoCache({ config, s: { db }, objectsClient, log: { warn: (m) => warned.push(m) }, fetchImpl: async () => refused });
++ out = await cache.cache(image);
++ assert.strictEqual(out, null);
++ assert.ok(warned.some((m) => /answered 400/.test(m)), 'a refused photo is logged, not silent');
++ console.log('photo cache: 960px first, the imageinfo URL as fallback, refusals logged');
++})().catch((e) => { console.error(e); process.exitCode = 1; });
ddiff --git a/test/place-photo-pass.test.js b/test/place-photo-pass.test.js
nnew file mode 100644
iindex 0000000..f3b208b
---- /dev/null
++++ b/test/place-photo-pass.test.js
@@@ -0,0 +1,40 @@
++'use strict';
++/**
++ * The places job's photo pass (11-08: on launch every photo copy was refused, so places were stored without photos):
++ * places with a Wikidata item and no photo are found, best ranked first; a found photo is stored with its credit; a
++ * place with none is marked tried and not asked again for a week.
++ */
++const assert = require('assert');
++const { boot, check, done } = require('./helpers/boot');
++const { seed } = require('./helpers/seed');
++const osm = require('../server/sources/osm');
++
++(async () => {
++ const t = await boot();
++ try {
++ await seed(t);
++ const city = await t.ctx.s.db.maybe('SELECT id FROM tours_cities ORDER BY id LIMIT 1');
++ const place = (id, name, wikidata) => ({ ...osm.placeFromElement({ type: 'node', id, lat: 47.5, lon: -122.3, tags: { tourism: 'attraction', name, wikidata } }, city.id, new Date().toISOString()) });
++ const a = await t.ctx.store.upsertPlace({ ...place(91001, 'Garden A', 'Q111'), rankScore: 900 });
++ const b = await t.ctx.store.upsertPlace({ ...place(91002, 'Garden B', 'Q222'), rankScore: 100 });
++ await t.ctx.store.upsertPlace({ ...place(91003, 'No item', undefined), rankScore: 999 });
++
++ await check('places without a photo and with a Wikidata item, best ranked first', async () => {
++ const bare = await t.ctx.store.placesWithoutPhoto({ limit: 200 });
++ const ours = bare.filter((r) => [a, b].includes(r.id)).map((r) => r.id);
++ assert.deepStrictEqual(ours, [a, b]);
++ assert.ok(!bare.some((r) => r.name === 'No item'), 'a place with no Wikidata item has no photo to find');
++ });
++
++ await check('a found photo is stored with its credit; a place with none is not asked again for a week', async () => {
++ assert.strictEqual(await t.ctx.store.setPlacePhoto(a, { imageUrl: 'https://openvibe.media/api/v2/tours/objects/obj_1', imageCredit: 'A photographer',
++ imageLicense: 'CC BY-SA 4.0', imageSourceUrl: 'https://commons.wikimedia.org/wiki/File:A.jpg', gallery: [] }), true);
++ assert.strictEqual(await t.ctx.store.setPlacePhoto(b, { id: b }), false);
++ const row = await t.ctx.s.db.maybe('SELECT image_url, image_credit, photo_tried_at FROM tours_places WHERE id = $1', [a]);
++ assert.strictEqual(row.image_credit, 'A photographer');
++ const bare = (await t.ctx.store.placesWithoutPhoto({ limit: 200 })).map((r) => r.id);
++ assert.ok(!bare.includes(a) && !bare.includes(b), 'neither is asked again this week');
++ });
++ } finally { await t.close(); }
++ done();
++})();