mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(maps): stop the photo endpoint answering 404 for photo-less places (#1727)
Opening one trip fires a 404 per place without a photo, 30 to 60 of them from one IP in seconds. CrowdSec and fail2ban read that as scanning; the reporter had three users banned within five minutes. A place having no photo is not an error and no longer answers as one. OSM places were also sent to Google as node:/way:/relation: ids, which can only return 400 INVALID_ARGUMENT — a billed round-trip and about 1.3s per place. getPlacePhoto already guarded coords: this way; the OSM prefixes now get the same treatment, as does getPlaceDetailsExpanded, which had no guard. The negative cache separates a place having no photo from a failed provider call instead of expiring both after five minutes and replaying the storm.
This commit is contained in:
@@ -160,6 +160,9 @@ export class MapsController {
|
||||
if (!placeId.startsWith('coords:') && this.maps.photosDisabled()) {
|
||||
return { photoUrl: null };
|
||||
}
|
||||
// A place with no photo resolves to the same { photoUrl: null } body. It is an
|
||||
// empty result, not a missing resource, and one 404 per photo-less place gets
|
||||
// the user's IP banned by any 404-rate IPS in front of TREK (#1727).
|
||||
try {
|
||||
return await this.maps.photo(user.id, placeId, parseFloat(lat as string), parseFloat(lng as string), name);
|
||||
} catch (err: unknown) {
|
||||
@@ -173,24 +176,37 @@ export class MapsController {
|
||||
placePhotoBytes(@Param('placeId') placeId: string, @Res() res: Response): void {
|
||||
const fp = this.maps.photoBytesPath(placeId);
|
||||
if (!fp) {
|
||||
res.status(404).json({ error: 'Photo not cached' });
|
||||
// Same reasoning as the JSON endpoint above, and the bigger half of #1727:
|
||||
// places keep this URL in image_url, so a trip render asks for one photo per
|
||||
// place and every entry the cache sweep dropped answered 404 — a whole map
|
||||
// worth of them from one IP in one second. An empty 204 leaves the <img>
|
||||
// with nothing to show, exactly like the 404 did.
|
||||
this.emptyPhoto(res);
|
||||
return;
|
||||
}
|
||||
// Stream the cached file directly instead of res.sendFile(): the send library
|
||||
// bundled under @nestjs/platform-express rejects absolute Windows paths (drive
|
||||
// letter, no `root`) with a NotFoundError that surfaced as an unhandled 500,
|
||||
// even though the file exists. A plain read stream serves the bytes
|
||||
// cross-platform; a read error still yields the legacy 404. Cached photos are
|
||||
// always JPEG (placePhotoCache writes `<hash>.jpg`).
|
||||
// cross-platform. Cached photos are always JPEG (placePhotoCache writes
|
||||
// `<hash>.jpg`).
|
||||
res.set('Cache-Control', 'public, max-age=2592000, immutable');
|
||||
res.type('image/jpeg');
|
||||
const stream = createReadStream(fp);
|
||||
stream.on('error', () => {
|
||||
if (!res.headersSent) res.status(404).json({ error: 'Photo not cached' });
|
||||
if (!res.headersSent) this.emptyPhoto(res);
|
||||
});
|
||||
stream.pipe(res);
|
||||
}
|
||||
|
||||
// 204 for "no bytes to serve". Overrides the immutable Cache-Control the hit
|
||||
// path already set — a photo that reappears in the cache must not stay hidden
|
||||
// behind a month-old empty response.
|
||||
private emptyPhoto(res: Response): void {
|
||||
res.set('Cache-Control', 'no-store');
|
||||
res.status(204).end();
|
||||
}
|
||||
|
||||
@Get('reverse')
|
||||
async reverse(
|
||||
@Query('lat') lat?: string,
|
||||
|
||||
@@ -61,9 +61,22 @@ interface GoogleAutocompleteSuggestion {
|
||||
};
|
||||
}
|
||||
|
||||
interface GoogleTimePoint {
|
||||
day?: number;
|
||||
hour?: number;
|
||||
minute?: number;
|
||||
}
|
||||
|
||||
interface GoogleOpeningHours {
|
||||
weekdayDescriptions?: string[];
|
||||
openNow?: boolean;
|
||||
periods?: { open?: GoogleTimePoint; close?: GoogleTimePoint }[];
|
||||
specialDays?: { date?: { year?: number; month?: number; day?: number } }[];
|
||||
}
|
||||
|
||||
interface GooglePlaceDetails extends GooglePlaceResult {
|
||||
userRatingCount?: number;
|
||||
regularOpeningHours?: { weekdayDescriptions?: string[]; openNow?: boolean };
|
||||
regularOpeningHours?: GoogleOpeningHours;
|
||||
editorialSummary?: { text: string };
|
||||
reviews?: {
|
||||
authorAttribution?: { displayName?: string; photoUri?: string };
|
||||
@@ -531,10 +544,107 @@ export async function searchOverpassPois(
|
||||
|
||||
// ── Opening hours parsing ────────────────────────────────────────────────────
|
||||
|
||||
export function parseOpeningHours(ohString: string): { weekdayDescriptions: string[]; openNow: boolean | null } {
|
||||
// Machine-readable opening hours, in Google's numbering: day 0 = Sunday … 6 = Saturday.
|
||||
// The weekday descriptions next to them are display text in the requested language and
|
||||
// cannot be parsed reliably; these can. A period without `close` is Google's way of
|
||||
// saying the place never closes.
|
||||
export interface OpeningTimePoint {
|
||||
day: number;
|
||||
hour: number;
|
||||
minute: number;
|
||||
}
|
||||
export interface OpeningPeriod {
|
||||
open: OpeningTimePoint;
|
||||
close: OpeningTimePoint | null;
|
||||
}
|
||||
|
||||
// Places (New) speaks proto3 JSON, which leaves out fields that hold the default
|
||||
// value — a period starting Sunday midnight arrives as `{}`, not as three zeroes.
|
||||
function toTimePoint(point: GoogleTimePoint | undefined): OpeningTimePoint | null {
|
||||
if (!point || typeof point !== 'object') return null;
|
||||
const field = (value: unknown): number | null => {
|
||||
if (value === undefined || value === null) return 0;
|
||||
return typeof value === 'number' && Number.isFinite(value) ? Math.trunc(value) : null;
|
||||
};
|
||||
const day = field(point.day);
|
||||
const hour = field(point.hour);
|
||||
const minute = field(point.minute);
|
||||
if (day === null || hour === null || minute === null) return null;
|
||||
if (day < 0 || day > 6 || hour < 0 || hour > 23 || minute < 0 || minute > 59) return null;
|
||||
return { day, hour, minute };
|
||||
}
|
||||
|
||||
export function normalizeOpeningPeriods(periods: GoogleOpeningHours['periods']): OpeningPeriod[] | null {
|
||||
if (!Array.isArray(periods) || periods.length === 0) return null;
|
||||
const result: OpeningPeriod[] = [];
|
||||
for (const period of periods) {
|
||||
const open = toTimePoint(period?.open);
|
||||
if (!open) continue;
|
||||
// A missing close means "never closes"; a close that is there but unusable makes
|
||||
// the whole period a guess, so it is dropped rather than read as round-the-clock.
|
||||
const close = period?.close == null ? null : toTimePoint(period.close);
|
||||
if (period?.close != null && !close) continue;
|
||||
result.push({ open, close });
|
||||
}
|
||||
return result.length > 0 ? result : null;
|
||||
}
|
||||
|
||||
// Holidays and other exceptional days that the weekly periods do not describe. Google
|
||||
// reports them as calendar dates; they travel to the client as YYYY-MM-DD so it can
|
||||
// compare them against the place's own local date.
|
||||
export function normalizeSpecialDays(specialDays: GoogleOpeningHours['specialDays']): string[] | null {
|
||||
if (!Array.isArray(specialDays) || specialDays.length === 0) return null;
|
||||
const dates: string[] = [];
|
||||
for (const entry of specialDays) {
|
||||
const date = entry?.date;
|
||||
if (!date) continue;
|
||||
const { year, month, day } = date;
|
||||
if (typeof year !== 'number' || typeof month !== 'number' || typeof day !== 'number') continue;
|
||||
if (month < 1 || month > 12 || day < 1 || day > 31) continue;
|
||||
const iso = `${String(year).padStart(4, '0')}-${String(month).padStart(2, '0')}-${String(day).padStart(2, '0')}`;
|
||||
if (!dates.includes(iso)) dates.push(iso);
|
||||
}
|
||||
return dates.length > 0 ? dates : null;
|
||||
}
|
||||
|
||||
// "09:00-18:00", also the "20:00-02:00" and "00:00-24:00" spellings OSM uses.
|
||||
const OSM_TIME_RANGE = /(\d{1,2}):(\d{2})\s*-\s*(\d{1,2}):(\d{2})/g;
|
||||
|
||||
/** Periods for one OSM weekday line. `dayIdx` is Monday-based, the output is not. */
|
||||
function osmPeriods(dayIdx: number, timePart: string): OpeningPeriod[] {
|
||||
const day = (dayIdx + 1) % 7;
|
||||
const periods: OpeningPeriod[] = [];
|
||||
for (const match of timePart.matchAll(OSM_TIME_RANGE)) {
|
||||
const openHour = parseInt(match[1], 10);
|
||||
const openMinute = parseInt(match[2], 10);
|
||||
let closeHour = parseInt(match[3], 10);
|
||||
const closeMinute = parseInt(match[4], 10);
|
||||
if (openHour > 23 || openMinute > 59 || closeHour > 24 || closeMinute > 59) continue;
|
||||
// OSM writes the end of a day as 24:00, a clock reading Google's numbering has no
|
||||
// hour 24 — that is midnight of the following day, same as any range that wraps.
|
||||
let nextDay = closeHour * 60 + closeMinute <= openHour * 60 + openMinute;
|
||||
if (closeHour === 24) {
|
||||
if (closeMinute !== 0) continue;
|
||||
closeHour = 0;
|
||||
nextDay = true;
|
||||
}
|
||||
periods.push({
|
||||
open: { day, hour: openHour, minute: openMinute },
|
||||
close: { day: nextDay ? (day + 1) % 7 : day, hour: closeHour, minute: closeMinute },
|
||||
});
|
||||
}
|
||||
return periods;
|
||||
}
|
||||
|
||||
export function parseOpeningHours(ohString: string): {
|
||||
weekdayDescriptions: string[];
|
||||
openNow: boolean | null;
|
||||
periods: OpeningPeriod[];
|
||||
} {
|
||||
const DAYS = ['Mo', 'Tu', 'We', 'Th', 'Fr', 'Sa', 'Su'];
|
||||
const LONG = ['Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday', 'Sunday'];
|
||||
const result: string[] = LONG.map((d) => `${d}: ?`);
|
||||
const periods: OpeningPeriod[] = [];
|
||||
|
||||
// Parse segments like "Mo-Fr 09:00-18:00; Sa 10:00-14:00"
|
||||
for (const segment of ohString.split(';')) {
|
||||
@@ -560,6 +670,7 @@ export function parseOpeningHours(ohString: string): { weekdayDescriptions: stri
|
||||
}
|
||||
for (const idx of dayIndices) {
|
||||
result[idx] = `${LONG[idx]}: ${timePart.trim()}`;
|
||||
periods.push(...osmPeriods(idx, timePart));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -583,7 +694,7 @@ export function parseOpeningHours(ohString: string): { weekdayDescriptions: stri
|
||||
/* best effort */
|
||||
}
|
||||
|
||||
return { weekdayDescriptions: result, openNow };
|
||||
return { weekdayDescriptions: result, openNow, periods };
|
||||
}
|
||||
|
||||
// ── Build standardized OSM details ───────────────────────────────────────────
|
||||
@@ -591,12 +702,14 @@ export function parseOpeningHours(ohString: string): { weekdayDescriptions: stri
|
||||
export function buildOsmDetails(tags: Record<string, string>, osmType: string, osmId: string) {
|
||||
let opening_hours: string[] | null = null;
|
||||
let open_now: boolean | null = null;
|
||||
let opening_periods: OpeningPeriod[] | null = null;
|
||||
if (tags.opening_hours) {
|
||||
const parsed = parseOpeningHours(tags.opening_hours);
|
||||
const hasData = parsed.weekdayDescriptions.some((line) => !line.endsWith('?'));
|
||||
if (hasData) {
|
||||
opening_hours = parsed.weekdayDescriptions;
|
||||
open_now = parsed.openNow;
|
||||
opening_periods = parsed.periods.length > 0 ? parsed.periods : null;
|
||||
}
|
||||
}
|
||||
return {
|
||||
@@ -604,6 +717,7 @@ export function buildOsmDetails(tags: Record<string, string>, osmType: string, o
|
||||
phone: tags['contact:phone'] || tags.phone || null,
|
||||
opening_hours,
|
||||
open_now,
|
||||
opening_periods,
|
||||
osm_url: `https://www.openstreetmap.org/${osmType}/${osmId}`,
|
||||
summary: tags.description || null,
|
||||
source: 'openstreetmap' as const,
|
||||
@@ -834,6 +948,21 @@ async function autocompleteNominatim(
|
||||
|
||||
// ── Place details (Google or OSM) ────────────────────────────────────────────
|
||||
|
||||
// Ids that can never resolve against the Google Places API: coordinate pseudo-ids
|
||||
// (right-click places, in both the coords: and the bare "lat,lng" spelling the
|
||||
// collection views send), OSM ids the client sends when a place has no
|
||||
// google_place_id, and the raw photo URLs legacy rows keep in image_url. Google
|
||||
// answers those with a billable 400 INVALID_ARGUMENT, so every lookup sorts them
|
||||
// out before the call and uses the OSM/Wikimedia path instead.
|
||||
const NON_GOOGLE_PLACE_ID = /^(?:coords|node|way|relation):|^https?:\/\/|^-?\d+(?:\.\d+)?,\s*-?\d+(?:\.\d+)?$/i;
|
||||
// The subset that still has a provider behind it — Overpass for details,
|
||||
// Wikimedia for photos.
|
||||
const OSM_PLACE_ID = /^(?:node|way|relation):/i;
|
||||
|
||||
export function isGooglePlaceId(placeId: string): boolean {
|
||||
return !NON_GOOGLE_PLACE_ID.test(placeId);
|
||||
}
|
||||
|
||||
export async function getPlaceDetails(
|
||||
userId: number,
|
||||
placeId: string,
|
||||
@@ -912,6 +1041,11 @@ export async function getPlaceDetails(
|
||||
phone: data.nationalPhoneNumber || null,
|
||||
opening_hours: data.regularOpeningHours?.weekdayDescriptions || null,
|
||||
open_now: data.regularOpeningHours?.openNow ?? null,
|
||||
// open_now is a snapshot Google took when this payload was fetched and it is cached
|
||||
// for days; the periods let the client recompute the state in the place's own
|
||||
// timezone, which the localised weekday lines above cannot do. Issue #1680.
|
||||
opening_periods: normalizeOpeningPeriods(data.regularOpeningHours?.periods),
|
||||
opening_special_days: normalizeSpecialDays(data.regularOpeningHours?.specialDays),
|
||||
google_maps_url: data.googleMapsUri || null,
|
||||
summary: null,
|
||||
reviews: [],
|
||||
@@ -935,7 +1069,16 @@ export async function getPlaceDetailsExpanded(
|
||||
placeId: string,
|
||||
lang?: string,
|
||||
refresh = false,
|
||||
): Promise<{ place: Record<string, unknown> }> {
|
||||
): Promise<{ place: Record<string, unknown> | null }> {
|
||||
// Reviews and the editorial summary only exist at Google, but the id does not
|
||||
// have to be a Google one — the client sends whatever the place carries. OSM ids
|
||||
// keep the details they do have (Overpass, via the plain lookup); coordinate
|
||||
// pseudo-ids and legacy image URLs have no details source at all. Neither may be
|
||||
// forwarded to Google, which bills the 400 INVALID_ARGUMENT it answers with.
|
||||
if (!isGooglePlaceId(placeId)) {
|
||||
return OSM_PLACE_ID.test(placeId) ? getPlaceDetails(userId, placeId, lang) : { place: null };
|
||||
}
|
||||
|
||||
const langKey = toApiLang(lang, 'de');
|
||||
const apiKey = getMapsKey(userId);
|
||||
if (!apiKey) throw Object.assign(new Error('Google Maps API key not configured'), { status: 400 });
|
||||
@@ -982,6 +1125,8 @@ export async function getPlaceDetailsExpanded(
|
||||
phone: data.nationalPhoneNumber || null,
|
||||
opening_hours: data.regularOpeningHours?.weekdayDescriptions || null,
|
||||
open_now: data.regularOpeningHours?.openNow ?? null,
|
||||
opening_periods: normalizeOpeningPeriods(data.regularOpeningHours?.periods),
|
||||
opening_special_days: normalizeSpecialDays(data.regularOpeningHours?.specialDays),
|
||||
google_maps_url: data.googleMapsUri || null,
|
||||
summary: data.editorialSummary?.text || null,
|
||||
reviews: (data.reviews || []).slice(0, 5).map((r: NonNullable<GooglePlaceDetails['reviews']>[number]) => ({
|
||||
@@ -1014,29 +1159,38 @@ export async function getPlacePhoto(
|
||||
lat: number,
|
||||
lng: number,
|
||||
name?: string,
|
||||
): Promise<{ photoUrl: string; attribution: string | null }> {
|
||||
): Promise<{ photoUrl: string | null; attribution: string | null }> {
|
||||
// Disk cache hit — serve immediately, no Google call
|
||||
const diskHit = placePhotoCache.get(placeId);
|
||||
if (diskHit) return { photoUrl: diskHit.photoUrl, attribution: diskHit.attribution };
|
||||
|
||||
// Recent error — don't hammer the API
|
||||
if (placePhotoCache.getErrored(placeId)) {
|
||||
throw Object.assign(new Error('(Cache) No photo available'), { status: 404 });
|
||||
}
|
||||
// "No photo for this place" is an empty result, not a missing resource: a trip
|
||||
// view asks for one photo per place, so answering each miss with a 404 makes a
|
||||
// normal itinerary render look like a 404 scan to fail2ban/CrowdSec and gets
|
||||
// the user's IP banned. Every miss below returns photoUrl: null instead — the
|
||||
// same shape the photos kill-switch already returns.
|
||||
const noPhoto = { photoUrl: null, attribution: null };
|
||||
|
||||
// Recent miss — don't hammer the API
|
||||
if (placePhotoCache.getErrored(placeId)) return noPhoto;
|
||||
|
||||
// Deduplicate concurrent requests for the same placeId
|
||||
const existing = placePhotoCache.getInFlight(placeId);
|
||||
if (existing) {
|
||||
const result = await existing;
|
||||
if (!result) throw Object.assign(new Error('(Cache) No photo available'), { status: 404 });
|
||||
if (!result) return noPhoto;
|
||||
return { photoUrl: `/api/maps/place-photo/${encodeURIComponent(placeId)}/bytes`, attribution: result.attribution };
|
||||
}
|
||||
|
||||
// Tells the two empty outcomes apart for the negative cache below: a place that
|
||||
// has no photo anywhere is worth remembering for a day, a provider that refused
|
||||
// or timed out only for a few minutes.
|
||||
let providerFailed = false;
|
||||
|
||||
const fetchPromise = (async (): Promise<{ filePath: string; attribution: string | null } | null> => {
|
||||
await acquirePhotoFetchSlot();
|
||||
try {
|
||||
const apiKey = getMapsKey(userId);
|
||||
const isCoordLookup = placeId.startsWith('coords:');
|
||||
|
||||
// Coordinate-based Wikipedia/Wikimedia lookup. Used for coordinate-only
|
||||
// (right-click) places and as a fallback when a Google place yields no photo,
|
||||
@@ -1050,21 +1204,25 @@ export async function getPlacePhoto(
|
||||
// Follow redirects manually so each hop (the image URL can 3xx to a CDN
|
||||
// host) is re-validated against the SSRF guard, not just the first URL.
|
||||
const imgRes = await safeFetchFollow(wiki.photoUrl, undefined, { bypassInternalIpAllowed: true });
|
||||
if (!imgRes.ok) return null;
|
||||
if (!imgRes.ok) {
|
||||
providerFailed = true;
|
||||
return null;
|
||||
}
|
||||
const bytes = Buffer.from(await imgRes.arrayBuffer());
|
||||
const cached = await placePhotoCache.put(placeId, bytes, wiki.attribution);
|
||||
return { filePath: cached.filePath, attribution: cached.attribution };
|
||||
} catch {
|
||||
providerFailed = true;
|
||||
return null;
|
||||
}
|
||||
};
|
||||
|
||||
// Google Places photo for a Google place_id. Returns null (without marking an
|
||||
// error) on any miss — no key, URL-shaped id, request rejected, no photos, or
|
||||
// a failed media download — so the caller can fall back to Wikimedia.
|
||||
// Google Places photo for a Google place_id. Returns null on any miss — no
|
||||
// key, request rejected, no photos, or a failed media download — so the
|
||||
// caller can fall back to Wikimedia; the misses that were Google's fault
|
||||
// flag providerFailed on the way out.
|
||||
const fetchGooglePhoto = async (): Promise<{ filePath: string; attribution: string | null } | null> => {
|
||||
// URL-shaped placeIds aren't Google IDs — legacy DBs may store raw photo URLs in image_url
|
||||
if (!apiKey || /^https?:\/\//i.test(placeId)) return null;
|
||||
if (!apiKey) return null;
|
||||
|
||||
// Fetch details to get the photo name
|
||||
const detailsRes = await googleFetch(
|
||||
@@ -1080,12 +1238,14 @@ export async function getPlacePhoto(
|
||||
const body = await detailsRes.text();
|
||||
if (!detailsRes.ok) {
|
||||
console.error('Google Places photo details error:', detailsRes.status, body.slice(0, 200));
|
||||
providerFailed = true;
|
||||
return null;
|
||||
}
|
||||
let details: GooglePlaceDetails & { error?: { message?: string } };
|
||||
try {
|
||||
details = body ? JSON.parse(body) : { photos: [] };
|
||||
} catch {
|
||||
providerFailed = true;
|
||||
return null;
|
||||
}
|
||||
if (!details.photos?.length) return null;
|
||||
@@ -1100,10 +1260,17 @@ export async function getPlacePhoto(
|
||||
`getPlacePhoto/media(${placeId})`,
|
||||
{ headers: { 'X-Goog-Api-Key': apiKey } },
|
||||
);
|
||||
if (!mediaRes.ok) return null;
|
||||
// The place does have a photo — only the download for it went wrong.
|
||||
if (!mediaRes.ok) {
|
||||
providerFailed = true;
|
||||
return null;
|
||||
}
|
||||
|
||||
const bytes = Buffer.from(await mediaRes.arrayBuffer());
|
||||
if (!bytes.length) return null;
|
||||
if (!bytes.length) {
|
||||
providerFailed = true;
|
||||
return null;
|
||||
}
|
||||
|
||||
const cached = await placePhotoCache.put(placeId, bytes, attribution);
|
||||
|
||||
@@ -1121,8 +1288,8 @@ export async function getPlacePhoto(
|
||||
|
||||
// Prefer the Google photo (higher quality); if Google yields nothing, fall
|
||||
// back to the same coordinate-based Wikipedia/OSM lookup that right-click
|
||||
// places use. Coordinate-only ids skip Google entirely.
|
||||
if (!isCoordLookup) {
|
||||
// places use. Ids Google cannot resolve skip it entirely.
|
||||
if (isGooglePlaceId(placeId)) {
|
||||
const googlePhoto = await fetchGooglePhoto();
|
||||
if (googlePhoto) return googlePhoto;
|
||||
}
|
||||
@@ -1130,7 +1297,7 @@ export async function getPlacePhoto(
|
||||
const fallback = await fetchWikimediaFallback();
|
||||
if (fallback) return fallback;
|
||||
|
||||
placePhotoCache.markError(placeId);
|
||||
placePhotoCache.markError(placeId, providerFailed ? 'provider-error' : 'no-photo');
|
||||
return null;
|
||||
} finally {
|
||||
releasePhotoFetchSlot();
|
||||
@@ -1140,7 +1307,7 @@ export async function getPlacePhoto(
|
||||
placePhotoCache.setInFlight(placeId, fetchPromise);
|
||||
|
||||
const result = await fetchPromise;
|
||||
if (!result) throw Object.assign(new Error('No photo available'), { status: 404 });
|
||||
if (!result) return noPhoto;
|
||||
return { photoUrl: `/api/maps/place-photo/${encodeURIComponent(placeId)}/bytes`, attribution: result.attribution };
|
||||
}
|
||||
|
||||
|
||||
@@ -121,8 +121,9 @@ async function enrichOne(tripId: string, userId: number, place: EnrichablePlace,
|
||||
WHERE id = ? AND trip_id = ?`,
|
||||
).run(gpid, gftid, str(match.address), str(match.website), str(match.phone), place.id, tripId);
|
||||
|
||||
// Photo is best-effort: Google often has none, and getPlacePhoto throws 404 in
|
||||
// that case — a missing photo must never abort the rest of the enrichment.
|
||||
// Photo is best-effort: Google often has none, in which case getPlacePhoto
|
||||
// resolves with photoUrl: null. A missing photo (or a provider outage, which
|
||||
// still throws) must never abort the rest of the enrichment.
|
||||
try {
|
||||
const photo = await getPlacePhoto(userId, gpid, place.lat, place.lng, place.name);
|
||||
if (photo?.photoUrl) {
|
||||
|
||||
@@ -10,7 +10,22 @@ import path from 'node:path';
|
||||
// Overridable for tests (mirrors the TREK_DB_FILE seam) so the suite never touches
|
||||
// the real uploads tree.
|
||||
const GOOGLE_PHOTO_DIR = readEnv().paths.placePhotoDir || path.join(__dirname, '../../uploads/photos/google');
|
||||
const ERROR_TTL = 5 * 60 * 1000;
|
||||
// How long a "no photo for this place" answer stays remembered. Nothing about it
|
||||
// changes until a photo appears upstream, so it is worth keeping: without it every
|
||||
// trip open replays the whole provider fan-out, one lookup per place. A day is
|
||||
// still short enough that a newly configured API key takes effect overnight.
|
||||
// Entries are dropped outright when the place goes away (removeIfUnreferenced /
|
||||
// sweepOrphans).
|
||||
const MISSING_TTL = 24 * 60 * 60 * 1000;
|
||||
// A provider call that failed says nothing about the place — only that the API was
|
||||
// unreachable or unhappy just now. It is remembered just long enough to stop a
|
||||
// screenful of markers from hammering an API that is down, and only in memory, so
|
||||
// a restart retries right away.
|
||||
const FAILURE_TTL = 5 * 60 * 1000;
|
||||
const recentFailures = new Map<string, number>();
|
||||
// Sweep expired failures once the map grows past this. Whatever survives the sweep
|
||||
// is younger than FAILURE_TTL and expires on its own shortly after.
|
||||
const FAILURE_SWEEP_AT = 500;
|
||||
|
||||
// Marker photos are displayed tiny — cap stored images so an oversized source
|
||||
// (e.g. a Wikimedia Commons full-res original) can't bloat the cache. Matches
|
||||
@@ -72,15 +87,36 @@ export function get(placeId: string): CachedPhoto | null {
|
||||
}
|
||||
|
||||
export function getErrored(placeId: string): boolean {
|
||||
const failedAt = recentFailures.get(placeId);
|
||||
if (failedAt !== undefined) {
|
||||
if (Date.now() - failedAt < FAILURE_TTL) return true;
|
||||
recentFailures.delete(placeId);
|
||||
}
|
||||
|
||||
const row = db
|
||||
.prepare('SELECT error_at FROM google_place_photo_meta WHERE place_id = ? AND error_at IS NOT NULL')
|
||||
.get(placeId) as { error_at: number } | undefined;
|
||||
|
||||
if (!row) return false;
|
||||
return Date.now() - row.error_at < ERROR_TTL;
|
||||
return Date.now() - row.error_at < MISSING_TTL;
|
||||
}
|
||||
|
||||
export function markError(placeId: string): void {
|
||||
// Remember that a lookup came back empty. 'no-photo' is the lasting answer — no
|
||||
// provider has an image for this place — and is persisted; 'provider-error' is a
|
||||
// failed attempt and is only held in memory for a few minutes.
|
||||
export function markError(placeId: string, kind: 'no-photo' | 'provider-error' = 'no-photo'): void {
|
||||
if (kind === 'provider-error') {
|
||||
if (recentFailures.size >= FAILURE_SWEEP_AT) {
|
||||
const cutoff = Date.now() - FAILURE_TTL;
|
||||
for (const [id, at] of recentFailures) {
|
||||
if (at <= cutoff) recentFailures.delete(id);
|
||||
}
|
||||
}
|
||||
recentFailures.set(placeId, Date.now());
|
||||
return;
|
||||
}
|
||||
|
||||
recentFailures.delete(placeId);
|
||||
knownOnDisk.delete(placeId);
|
||||
db.prepare(
|
||||
'INSERT OR REPLACE INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, NULL, ?, ?)',
|
||||
@@ -111,6 +147,7 @@ export async function put(placeId: string, bytes: Buffer, attribution: string |
|
||||
await fsPromises.rename(tmp, fp);
|
||||
|
||||
knownOnDisk.add(placeId);
|
||||
recentFailures.delete(placeId);
|
||||
|
||||
db.prepare(
|
||||
'INSERT OR REPLACE INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, ?, ?, NULL)',
|
||||
|
||||
@@ -265,17 +265,45 @@ describe('Maps happy paths (mocked service)', () => {
|
||||
it('MAPS-004 — getPlacePhoto error with status returns that status', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
vi.mocked(mapsService.getPlacePhoto).mockRejectedValueOnce(
|
||||
Object.assign(new Error('Photo not found'), { status: 404 })
|
||||
Object.assign(new Error('Photo provider unavailable'), { status: 503 })
|
||||
);
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/maps/place-photo/some-place-id?lat=48.8&lng=2.3')
|
||||
.set('Cookie', authCookie(user.id));
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.status).toBe(503);
|
||||
expect(res.body).toHaveProperty('error');
|
||||
});
|
||||
|
||||
// A photo-less place must answer 200 { photoUrl: null }: one 404 per place turns
|
||||
// a normal itinerary render into a 404 storm that fail2ban/CrowdSec bans (#1727).
|
||||
it('MAPS-004b — a place with no photo answers 200 with photoUrl null', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
vi.mocked(mapsService.getPlacePhoto).mockResolvedValueOnce({ photoUrl: null, attribution: null });
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/maps/place-photo/node%3A5255005321?lat=36.76&lng=-3.84&name=Nerja')
|
||||
.set('Cookie', authCookie(user.id));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({ photoUrl: null, attribution: null });
|
||||
});
|
||||
|
||||
// Places keep the bytes URL in image_url, so an evicted cache entry means one
|
||||
// request per place on the next trip render — the second half of the same ban
|
||||
// vector. Nothing to serve is an empty 204, not a 404.
|
||||
it('MAPS-004c — the bytes proxy answers 204 for a photo that is not cached', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
|
||||
const res = await request(app)
|
||||
.get('/api/maps/place-photo/ChIJnot-cached/bytes')
|
||||
.set('Cookie', authCookie(user.id));
|
||||
|
||||
expect(res.status).toBe(204);
|
||||
expect(res.text).toBeFalsy();
|
||||
});
|
||||
|
||||
it('MAPS-005 — reverseGeocode error returns null values', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
vi.mocked(mapsService.reverseGeocode).mockRejectedValueOnce(new Error('Geocode failed'));
|
||||
|
||||
@@ -210,12 +210,20 @@ describe('MapsController (parity with the legacy /api/maps route)', () => {
|
||||
});
|
||||
|
||||
it('maps a 4xx service error', async () => {
|
||||
const photo = vi.fn().mockRejectedValue(withError(404, 'No photo available'));
|
||||
const photo = vi.fn().mockRejectedValue(withError(429, 'Rate limited'));
|
||||
expect(await thrown(() => makeController({ photosDisabled: () => false, photo }).placePhoto(user, 'p1', '1', '2'))).toEqual({
|
||||
status: 404, body: { error: 'No photo available' },
|
||||
status: 429, body: { error: 'Rate limited' },
|
||||
});
|
||||
});
|
||||
|
||||
// A place without a photo is an empty result, not a 404 — one 404 per photo-less
|
||||
// place gets the user banned by any 404-rate IPS in front of TREK (#1727).
|
||||
it('passes a photo-less place through as a 200 with photoUrl null', async () => {
|
||||
const photo = vi.fn().mockResolvedValue({ photoUrl: null, attribution: null });
|
||||
const res = await makeController({ photosDisabled: () => false, photo }).placePhoto(user, 'node:123', '1', '2');
|
||||
expect(res).toEqual({ photoUrl: null, attribution: null });
|
||||
});
|
||||
|
||||
it('logs and maps a 5xx service error', async () => {
|
||||
const photo = vi.fn().mockRejectedValue(withError(502, 'Upstream failed'));
|
||||
expect(await thrown(() => makeController({ photosDisabled: () => false, photo }).placePhoto(user, 'p1', '1', '2'))).toEqual({
|
||||
@@ -244,17 +252,22 @@ describe('MapsController (parity with the legacy /api/maps route)', () => {
|
||||
json: vi.fn(),
|
||||
set: vi.fn(),
|
||||
type: vi.fn(),
|
||||
end: vi.fn(),
|
||||
};
|
||||
return res as unknown as Response & { status: ReturnType<typeof vi.fn>; json: ReturnType<typeof vi.fn>; set: ReturnType<typeof vi.fn>; type: ReturnType<typeof vi.fn> };
|
||||
return res as unknown as Response & { status: ReturnType<typeof vi.fn>; json: ReturnType<typeof vi.fn>; set: ReturnType<typeof vi.fn>; type: ReturnType<typeof vi.fn>; end: ReturnType<typeof vi.fn> };
|
||||
}
|
||||
|
||||
beforeEach(() => createReadStream.mockReset());
|
||||
|
||||
it('404 when the photo is not cached', () => {
|
||||
// Places persist this URL in image_url, so an evicted cache entry means one
|
||||
// request per place on a trip render. 404 for each of them is the ban vector
|
||||
// from #1727 — an uncached photo answers 204 with an empty body instead.
|
||||
it('204 without a body when the photo is not cached', () => {
|
||||
const res = makeRes();
|
||||
makeController({ photoBytesPath: () => null }).placePhotoBytes('p1', res);
|
||||
expect(res.status).toHaveBeenCalledWith(404);
|
||||
expect(res.json).toHaveBeenCalledWith({ error: 'Photo not cached' });
|
||||
expect(res.status).toHaveBeenCalledWith(204);
|
||||
expect(res.end).toHaveBeenCalled();
|
||||
expect(res.json).not.toHaveBeenCalled();
|
||||
expect(createReadStream).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -269,18 +282,22 @@ describe('MapsController (parity with the legacy /api/maps route)', () => {
|
||||
expect(stream.pipe).toHaveBeenCalledWith(res);
|
||||
});
|
||||
|
||||
it('falls back to 404 when the read stream errors', () => {
|
||||
it('falls back to an empty 204 when the read stream errors', () => {
|
||||
let onError: () => void = () => {};
|
||||
const stream = { on: vi.fn((ev: string, cb: () => void) => { if (ev === 'error') onError = cb; return stream; }), pipe: vi.fn() };
|
||||
createReadStream.mockReturnValue(stream);
|
||||
const res = makeRes();
|
||||
makeController({ photoBytesPath: () => '/cache/p1.jpg' }).placePhotoBytes('p1', res);
|
||||
onError();
|
||||
expect(res.status).toHaveBeenCalledWith(404);
|
||||
expect(res.json).toHaveBeenCalledWith({ error: 'Photo not cached' });
|
||||
expect(res.status).toHaveBeenCalledWith(204);
|
||||
expect(res.end).toHaveBeenCalled();
|
||||
// The hit path already asked for a month of immutable caching — that header
|
||||
// must not survive onto the empty answer, or the photo stays hidden once it
|
||||
// is back in the cache.
|
||||
expect(res.set).toHaveBeenLastCalledWith('Cache-Control', 'no-store');
|
||||
});
|
||||
|
||||
it('does not re-send a 404 when the stream errors after headers were flushed', () => {
|
||||
it('does not re-send a 204 when the stream errors after headers were flushed', () => {
|
||||
let onError: () => void = () => {};
|
||||
const stream = { on: vi.fn((ev: string, cb: () => void) => { if (ev === 'error') onError = cb; return stream; }), pipe: vi.fn() };
|
||||
createReadStream.mockReturnValue(stream);
|
||||
@@ -289,7 +306,7 @@ describe('MapsController (parity with the legacy /api/maps route)', () => {
|
||||
makeController({ photoBytesPath: () => '/cache/p1.jpg' }).placePhotoBytes('p1', res);
|
||||
onError();
|
||||
expect(res.status).not.toHaveBeenCalled();
|
||||
expect(res.json).not.toHaveBeenCalled();
|
||||
expect(res.end).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -8,9 +8,12 @@
|
||||
*/
|
||||
import {
|
||||
parseOpeningHours,
|
||||
normalizeOpeningPeriods,
|
||||
normalizeSpecialDays,
|
||||
buildOsmDetails,
|
||||
getMapsKey,
|
||||
googleFtidFromMapsUrl,
|
||||
isGooglePlaceId,
|
||||
buildUserAgent,
|
||||
resolveOverpassEndpoints,
|
||||
resolveOverpassTimeoutMs,
|
||||
@@ -25,6 +28,7 @@ const {
|
||||
mockCheckSsrf,
|
||||
mockCacheGet,
|
||||
mockCacheGetErrored,
|
||||
mockCacheMarkError,
|
||||
mockCachePut,
|
||||
mockCacheGetInFlight,
|
||||
mockCacheSetInFlight,
|
||||
@@ -34,6 +38,7 @@ const {
|
||||
mockCheckSsrf: vi.fn(async () => ({ allowed: true })),
|
||||
mockCacheGet: vi.fn(() => null as any),
|
||||
mockCacheGetErrored: vi.fn(() => false),
|
||||
mockCacheMarkError: vi.fn(),
|
||||
mockCachePut: vi.fn(async (placeId: string, _bytes: Buffer, attribution: string | null) => ({
|
||||
photoUrl: `/api/maps/place-photo/${encodeURIComponent(placeId)}/bytes`,
|
||||
filePath: `/tmp/${placeId}.jpg`,
|
||||
@@ -83,7 +88,7 @@ vi.mock('../../../src/services/placePhotoCache', () => ({
|
||||
get: (placeId: string) => mockCacheGet(placeId),
|
||||
getErrored: (placeId: string) => mockCacheGetErrored(placeId),
|
||||
put: (placeId: string, bytes: Buffer, attribution: string | null) => mockCachePut(placeId, bytes, attribution),
|
||||
markError: vi.fn(),
|
||||
markError: (placeId: string, kind?: string) => mockCacheMarkError(placeId, kind),
|
||||
getInFlight: (placeId: string) => mockCacheGetInFlight(placeId),
|
||||
setInFlight: (placeId: string, p: Promise<any>) => mockCacheSetInFlight(placeId, p),
|
||||
serveFilePath: vi.fn(() => null),
|
||||
@@ -100,6 +105,7 @@ afterEach(() => {
|
||||
mockCacheGet.mockReturnValue(null);
|
||||
mockCacheGetErrored.mockReset();
|
||||
mockCacheGetErrored.mockReturnValue(false);
|
||||
mockCacheMarkError.mockReset();
|
||||
mockCachePut.mockReset();
|
||||
mockCachePut.mockImplementation(async (placeId: string, _bytes: Buffer, attribution: string | null) => ({
|
||||
photoUrl: `/api/maps/place-photo/${encodeURIComponent(placeId)}/bytes`,
|
||||
@@ -157,6 +163,84 @@ describe('parseOpeningHours', () => {
|
||||
const elapsed = Date.now() - start;
|
||||
expect(elapsed).toBeLessThan(100);
|
||||
});
|
||||
|
||||
it('MAPS-007b: emits machine-readable periods in Google day numbering (Sunday = 0)', () => {
|
||||
const result = parseOpeningHours('Mo 09:00-18:00; Su 10:00-14:00');
|
||||
expect(result.periods).toEqual([
|
||||
{ open: { day: 1, hour: 9, minute: 0 }, close: { day: 1, hour: 18, minute: 0 } },
|
||||
{ open: { day: 0, hour: 10, minute: 0 }, close: { day: 0, hour: 14, minute: 0 } },
|
||||
]);
|
||||
});
|
||||
|
||||
it('MAPS-007c: a period past midnight closes on the following day', () => {
|
||||
const result = parseOpeningHours('Sa 20:00-02:00');
|
||||
expect(result.periods).toEqual([
|
||||
{ open: { day: 6, hour: 20, minute: 0 }, close: { day: 0, hour: 2, minute: 0 } },
|
||||
]);
|
||||
});
|
||||
|
||||
it('MAPS-007d: the OSM 24:00 spelling becomes midnight of the next day', () => {
|
||||
// Google's clock has no hour 24, so "00:00-24:00" is a full day, not a rejected range.
|
||||
const result = parseOpeningHours('Mo 00:00-24:00');
|
||||
expect(result.periods).toEqual([
|
||||
{ open: { day: 1, hour: 0, minute: 0 }, close: { day: 2, hour: 0, minute: 0 } },
|
||||
]);
|
||||
});
|
||||
|
||||
it('MAPS-007e: unusable clock values produce no period', () => {
|
||||
expect(parseOpeningHours('Mo 09:75-18:00').periods).toEqual([]);
|
||||
expect(parseOpeningHours('Mo 09:00-24:30').periods).toEqual([]);
|
||||
expect(parseOpeningHours('Mo closed').periods).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
// ── normalizeOpeningPeriods / normalizeSpecialDays ────────────────────────────
|
||||
|
||||
describe('normalizeOpeningPeriods', () => {
|
||||
it('MAPS-007f: keeps well-formed periods and fills the zeroes proto3 JSON omits', () => {
|
||||
expect(
|
||||
normalizeOpeningPeriods([{ open: { day: 3, hour: 9, minute: 30 }, close: { day: 3, hour: 17 } }]),
|
||||
).toEqual([{ open: { day: 3, hour: 9, minute: 30 }, close: { day: 3, hour: 17, minute: 0 } }]);
|
||||
// Sunday midnight arrives as an empty object.
|
||||
expect(normalizeOpeningPeriods([{ open: {} }])).toEqual([{ open: { day: 0, hour: 0, minute: 0 }, close: null }]);
|
||||
});
|
||||
|
||||
it('MAPS-007g: a period without a close survives as the round-the-clock marker', () => {
|
||||
expect(normalizeOpeningPeriods([{ open: { day: 0, hour: 0, minute: 0 } }])).toEqual([
|
||||
{ open: { day: 0, hour: 0, minute: 0 }, close: null },
|
||||
]);
|
||||
});
|
||||
|
||||
it('MAPS-007h: drops out-of-range points and periods with an unusable close', () => {
|
||||
expect(normalizeOpeningPeriods([{ open: { day: 7, hour: 9, minute: 0 } }])).toBeNull();
|
||||
expect(normalizeOpeningPeriods([{ open: { day: 1, hour: 24, minute: 0 } }])).toBeNull();
|
||||
// A broken close must not be mistaken for "never closes".
|
||||
expect(
|
||||
normalizeOpeningPeriods([{ open: { day: 1, hour: 9, minute: 0 }, close: { day: 1, hour: 9, minute: 90 } }]),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('MAPS-007i: returns null when there is nothing to normalise', () => {
|
||||
expect(normalizeOpeningPeriods(undefined)).toBeNull();
|
||||
expect(normalizeOpeningPeriods([])).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('normalizeSpecialDays', () => {
|
||||
it('MAPS-007j: turns Google date parts into ISO dates without duplicates', () => {
|
||||
expect(
|
||||
normalizeSpecialDays([
|
||||
{ date: { year: 2026, month: 12, day: 25 } },
|
||||
{ date: { year: 2026, month: 1, day: 1 } },
|
||||
{ date: { year: 2026, month: 12, day: 25 } },
|
||||
]),
|
||||
).toEqual(['2026-12-25', '2026-01-01']);
|
||||
});
|
||||
|
||||
it('MAPS-007k: skips incomplete or impossible dates and returns null when none are left', () => {
|
||||
expect(normalizeSpecialDays([{}, { date: { year: 2026, month: 13, day: 1 } }, { date: { year: 2026, day: 5 } }])).toBeNull();
|
||||
expect(normalizeSpecialDays(undefined)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// ── buildOsmDetails ───────────────────────────────────────────────────────────
|
||||
@@ -202,6 +286,19 @@ describe('buildOsmDetails', () => {
|
||||
expect(buildOsmDetails({}, 'node', '1').source).toBe('openstreetmap');
|
||||
});
|
||||
|
||||
it('MAPS-012b: carries the structured periods next to the weekday lines', () => {
|
||||
const result = buildOsmDetails({ opening_hours: 'Mo-Tu 09:00-18:00' }, 'node', '1');
|
||||
expect(result.opening_periods).toEqual([
|
||||
{ open: { day: 1, hour: 9, minute: 0 }, close: { day: 1, hour: 18, minute: 0 } },
|
||||
{ open: { day: 2, hour: 9, minute: 0 }, close: { day: 2, hour: 18, minute: 0 } },
|
||||
]);
|
||||
});
|
||||
|
||||
it('MAPS-012c: opening_periods is null when the day lines carry no usable times', () => {
|
||||
expect(buildOsmDetails({}, 'node', '1').opening_periods).toBeNull();
|
||||
expect(buildOsmDetails({ opening_hours: 'Mo closed' }, 'node', '1').opening_periods).toBeNull();
|
||||
});
|
||||
|
||||
it('MAPS-014b: opening_hours is null when all days have unknown times (all "?")', () => {
|
||||
// "closed" does not match the day+time pattern so all days remain "?"
|
||||
const result = buildOsmDetails({ opening_hours: 'closed' }, 'node', '1');
|
||||
@@ -1488,6 +1585,71 @@ describe('getPlaceDetails (fetch stubbed)', () => {
|
||||
expect((result.place as any).open_now).toBe(false);
|
||||
});
|
||||
|
||||
it('MAPS-041f2: hands the structured opening periods and special days to the client', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
id: 'ChIJPeriods',
|
||||
regularOpeningHours: {
|
||||
// Localised display text — the client shows it but must not parse it.
|
||||
weekdayDescriptions: ['月曜日: 9:00~18:00'],
|
||||
openNow: true,
|
||||
periods: [{ open: { day: 1, hour: 9, minute: 0 }, close: { day: 1, hour: 18, minute: 0 } }],
|
||||
specialDays: [{ date: { year: 2026, month: 12, day: 25 } }],
|
||||
},
|
||||
}),
|
||||
}),
|
||||
);
|
||||
const { getPlaceDetails } = await import('../../../src/services/mapsService');
|
||||
const place = (await getPlaceDetails(1, 'ChIJPeriods')).place as any;
|
||||
expect(place.opening_periods).toEqual([
|
||||
{ open: { day: 1, hour: 9, minute: 0 }, close: { day: 1, hour: 18, minute: 0 } },
|
||||
]);
|
||||
expect(place.opening_special_days).toEqual(['2026-12-25']);
|
||||
expect(place.opening_hours).toEqual(['月曜日: 9:00~18:00']);
|
||||
});
|
||||
|
||||
it('MAPS-041f3: opening_periods is null when Google sends no periods', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
id: 'ChIJNoPeriods',
|
||||
regularOpeningHours: { weekdayDescriptions: ['Monday: 9:00 AM – 5:00 PM'], openNow: false },
|
||||
}),
|
||||
}),
|
||||
);
|
||||
const { getPlaceDetails } = await import('../../../src/services/mapsService');
|
||||
const place = (await getPlaceDetails(1, 'ChIJNoPeriods')).place as any;
|
||||
expect(place.opening_periods).toBeNull();
|
||||
expect(place.opening_special_days).toBeNull();
|
||||
});
|
||||
|
||||
it('MAPS-041f4: getPlaceDetailsExpanded carries the periods too', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
// expanded=1 cache miss
|
||||
mockDbGet.mockReturnValueOnce(undefined);
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
id: 'ChIJExpandedPeriods',
|
||||
// A place that never closes: one period, no close point.
|
||||
regularOpeningHours: { openNow: true, periods: [{ open: { day: 0, hour: 0, minute: 0 } }] },
|
||||
}),
|
||||
}),
|
||||
);
|
||||
const { getPlaceDetailsExpanded } = await import('../../../src/services/mapsService');
|
||||
const place = (await getPlaceDetailsExpanded(1, 'ChIJExpandedPeriods')).place as any;
|
||||
expect(place.opening_periods).toEqual([{ open: { day: 0, hour: 0, minute: 0 }, close: null }]);
|
||||
});
|
||||
|
||||
it('MAPS-041g: getPlaceDetailsExpanded truncates reviews to first 5 entries', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
// expanded=1 cache miss
|
||||
@@ -1509,6 +1671,33 @@ describe('getPlaceDetails (fetch stubbed)', () => {
|
||||
const result = await getPlaceDetailsExpanded(1, 'ChIJMany');
|
||||
expect((result.place as any).reviews).toHaveLength(5);
|
||||
});
|
||||
|
||||
// The client sends whatever id the place carries, and the expanded lookup used to
|
||||
// forward all of them — including OSM ids — to Google, which bills the 400
|
||||
// INVALID_ARGUMENT it answers with (#1727).
|
||||
it('MAPS-041h: getPlaceDetailsExpanded serves an OSM id from Overpass instead of Google', async () => {
|
||||
mockDbGet.mockReturnValue({ maps_api_key: 'gkey' });
|
||||
const fetchMock = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({ elements: [{ tags: { website: 'https://nerja.example' } }] }),
|
||||
});
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const { getPlaceDetailsExpanded } = await import('../../../src/services/mapsService');
|
||||
const result = await getPlaceDetailsExpanded(1, 'node:5255005321');
|
||||
expect((result.place as any).source).toBe('openstreetmap');
|
||||
expect((result.place as any).website).toBe('https://nerja.example');
|
||||
expect(fetchMock.mock.calls.map((call) => String(call[0])).some((url) => url.includes('places.googleapis.com')))
|
||||
.toBe(false);
|
||||
});
|
||||
|
||||
it('MAPS-041i: getPlaceDetailsExpanded answers a coordinate pseudo-id with no place at all', async () => {
|
||||
mockDbGet.mockReturnValue({ maps_api_key: 'gkey' });
|
||||
const fetchMock = vi.fn();
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const { getPlaceDetailsExpanded } = await import('../../../src/services/mapsService');
|
||||
await expect(getPlaceDetailsExpanded(1, 'coords:48.8,2.3')).resolves.toEqual({ place: null });
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
// ── getPlacePhoto (fetch stubbed) ────────────────────────────────────────────
|
||||
@@ -1539,7 +1728,7 @@ describe('getPlacePhoto (fetch stubbed)', () => {
|
||||
expect(mockCachePut).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it('MAPS-043: throws 404 when Wikimedia returns nothing and no API key', async () => {
|
||||
it('MAPS-043: resolves with photoUrl null when Wikimedia returns nothing and no API key', async () => {
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi.fn().mockResolvedValue({
|
||||
@@ -1548,7 +1737,10 @@ describe('getPlacePhoto (fetch stubbed)', () => {
|
||||
}),
|
||||
);
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
await expect(getPlacePhoto(999, 'coords:0.0,0.0', 0, 0)).rejects.toMatchObject({ status: 404 });
|
||||
await expect(getPlacePhoto(999, 'coords:0.0,0.0', 0, 0)).resolves.toEqual({
|
||||
photoUrl: null,
|
||||
attribution: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('MAPS-043b: returns cached photo when disk cache returns a hit', async () => {
|
||||
@@ -1567,27 +1759,42 @@ describe('getPlacePhoto (fetch stubbed)', () => {
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('MAPS-043c: throws 404 from error cache without making a network request', async () => {
|
||||
it('MAPS-043c: serves the negative cache with photoUrl null and no network request', async () => {
|
||||
mockCacheGetErrored.mockReturnValue(true);
|
||||
const fetchMock = vi.fn();
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
const errorId = `coords:error-cache-${Date.now()}`;
|
||||
await expect(getPlacePhoto(999, errorId, 0, 0)).rejects.toMatchObject({ status: 404 });
|
||||
await expect(getPlacePhoto(999, errorId, 0, 0)).resolves.toEqual({ photoUrl: null, attribution: null });
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('MAPS-043d: throws 404 when lat/lng are NaN and no API key', async () => {
|
||||
it('MAPS-043d: resolves with photoUrl null when lat/lng are NaN and no API key', async () => {
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
const nanId = `coords:nan-test-${Date.now()}`;
|
||||
await expect(getPlacePhoto(999, nanId, NaN, NaN)).rejects.toMatchObject({ status: 404 });
|
||||
await expect(getPlacePhoto(999, nanId, NaN, NaN)).resolves.toEqual({ photoUrl: null, attribution: null });
|
||||
});
|
||||
|
||||
it('MAPS-043e: falls through and throws 404 when Wikimedia fetch throws', async () => {
|
||||
it('MAPS-043e: falls through to photoUrl null when the Wikimedia fetch throws', async () => {
|
||||
vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network fail')));
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
const throwId = `coords:throw-test-${Date.now()}`;
|
||||
await expect(getPlacePhoto(999, throwId, 48.8, 2.3, 'Place')).rejects.toMatchObject({ status: 404 });
|
||||
await expect(getPlacePhoto(999, throwId, 48.8, 2.3, 'Place')).resolves.toEqual({
|
||||
photoUrl: null,
|
||||
attribution: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('MAPS-043f: an in-flight lookup that found nothing resolves the waiter with photoUrl null', async () => {
|
||||
mockCacheGetInFlight.mockReturnValue(Promise.resolve(null) as any);
|
||||
const fetchMock = vi.fn();
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
await expect(getPlacePhoto(999, 'ChIJInFlight', 48.8, 2.3)).resolves.toEqual({
|
||||
photoUrl: null,
|
||||
attribution: null,
|
||||
});
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('MAPS-044: returns proxy URL via Google path when API key present and photos exist', async () => {
|
||||
@@ -1616,7 +1823,7 @@ describe('getPlacePhoto (fetch stubbed)', () => {
|
||||
expect(mockCachePut).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it('MAPS-044b: throws 404 when Google details fetch returns non-ok', async () => {
|
||||
it('MAPS-044b: resolves with photoUrl null when Google details fetch returns non-ok', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
@@ -1628,10 +1835,50 @@ describe('getPlacePhoto (fetch stubbed)', () => {
|
||||
);
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
const errId = `ChIJErr-${Date.now()}`;
|
||||
await expect(getPlacePhoto(1, errId, 48.8, 2.3)).rejects.toMatchObject({ status: 404 });
|
||||
await expect(getPlacePhoto(1, errId, 48.8, 2.3)).resolves.toEqual({ photoUrl: null, attribution: null });
|
||||
// A rejected provider call says nothing about the place — remember it as a
|
||||
// failed attempt (minutes), not as "this place has no photo" (a day).
|
||||
expect(mockCacheMarkError).toHaveBeenCalledWith(errId, 'provider-error');
|
||||
});
|
||||
|
||||
it('MAPS-044c: throws 404 when Google place has no photos', async () => {
|
||||
it('MAPS-044b2: remembers a place both providers came up empty for as a lasting miss', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi
|
||||
.fn()
|
||||
// Google answers, the place simply has no photos.
|
||||
.mockResolvedValueOnce({ ok: true, text: async () => JSON.stringify({ photos: [] }) })
|
||||
// Wikimedia has no article near the coordinates either.
|
||||
.mockResolvedValueOnce({ ok: true, json: async () => ({ query: { pages: {} } }) }),
|
||||
);
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
const emptyId = `ChIJEmpty-${Date.now()}`;
|
||||
await expect(getPlacePhoto(1, emptyId, 48.8, 2.3)).resolves.toEqual({ photoUrl: null, attribution: null });
|
||||
expect(mockCacheMarkError).toHaveBeenCalledWith(emptyId, 'no-photo');
|
||||
});
|
||||
|
||||
it('MAPS-044b3: a failed Wikimedia image download counts as a failed attempt', async () => {
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: async () => ({ query: { pages: { '1': { thumbnail: { source: 'https://wiki.org/photo.jpg' } } } } }),
|
||||
})
|
||||
.mockResolvedValueOnce({ ok: false, status: 503, arrayBuffer: async () => new ArrayBuffer(0) }),
|
||||
);
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
const downId = `coords:down-${Date.now()}`;
|
||||
await expect(getPlacePhoto(999, downId, 48.8, 2.3, 'Place')).resolves.toEqual({
|
||||
photoUrl: null,
|
||||
attribution: null,
|
||||
});
|
||||
expect(mockCacheMarkError).toHaveBeenCalledWith(downId, 'provider-error');
|
||||
});
|
||||
|
||||
it('MAPS-044c: resolves with photoUrl null when the Google place has no photos', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
vi.stubGlobal(
|
||||
'fetch',
|
||||
@@ -1642,10 +1889,10 @@ describe('getPlacePhoto (fetch stubbed)', () => {
|
||||
);
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
const noPhotoId = `ChIJNone-${Date.now()}`;
|
||||
await expect(getPlacePhoto(1, noPhotoId, 48.8, 2.3)).rejects.toMatchObject({ status: 404 });
|
||||
await expect(getPlacePhoto(1, noPhotoId, 48.8, 2.3)).resolves.toEqual({ photoUrl: null, attribution: null });
|
||||
});
|
||||
|
||||
it('MAPS-044d: throws 404 when media endpoint returns non-ok status', async () => {
|
||||
it('MAPS-044d: resolves with photoUrl null when the media endpoint returns non-ok status', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
@@ -1664,7 +1911,7 @@ describe('getPlacePhoto (fetch stubbed)', () => {
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
const noUriId = `ChIJXYZ-${Date.now()}`;
|
||||
await expect(getPlacePhoto(1, noUriId, 48.8, 2.3)).rejects.toMatchObject({ status: 404 });
|
||||
await expect(getPlacePhoto(1, noUriId, 48.8, 2.3)).resolves.toEqual({ photoUrl: null, attribution: null });
|
||||
});
|
||||
|
||||
it('MAPS-044e: returns proxy URL with null attribution when authorAttributions is empty', async () => {
|
||||
@@ -1748,6 +1995,58 @@ describe('getPlacePhoto (fetch stubbed)', () => {
|
||||
expect(result.attribution).toBe('Wikipedia');
|
||||
expect(mockCachePut).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
// OSM ids are what the client sends whenever a place has no google_place_id.
|
||||
// Google rejects them with a billable 400 INVALID_ARGUMENT, so they must go
|
||||
// straight to the Wikimedia fallback like coords: ids already do.
|
||||
it.each(['node:5255005321', 'way:84527326', 'relation:345407'])(
|
||||
'MAPS-044h: skips Google for the OSM id %s even when an API key is configured',
|
||||
async (osmId) => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: async () => ({ query: { pages: { '1': { thumbnail: { source: 'https://wiki.org/osm.jpg' } } } } }),
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
arrayBuffer: async () => new ArrayBuffer(90),
|
||||
});
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
const result = await getPlacePhoto(1, osmId, 36.7617, -3.8448, 'Cueva de Nerja');
|
||||
expect(result.photoUrl).toBe(`/api/maps/place-photo/${encodeURIComponent(osmId)}/bytes`);
|
||||
const requested = fetchMock.mock.calls.map((call) => String(call[0]));
|
||||
expect(requested.some((url) => url.includes('places.googleapis.com'))).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it('MAPS-044i: an OSM id with no Wikimedia hit resolves with photoUrl null instead of a 404', async () => {
|
||||
mockDbGet.mockReturnValueOnce({ maps_api_key: 'gkey' });
|
||||
const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: async () => ({ query: { pages: {} } }) });
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const { getPlacePhoto } = await import('../../../src/services/mapsService');
|
||||
await expect(getPlacePhoto(1, 'node:2481346642', 36.76, -3.84, 'Nerja')).resolves.toEqual({
|
||||
photoUrl: null,
|
||||
attribution: null,
|
||||
});
|
||||
expect(fetchMock.mock.calls.map((call) => String(call[0])).some((url) => url.includes('places.googleapis.com')))
|
||||
.toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isGooglePlaceId', () => {
|
||||
it('MAPS-045: accepts Google place ids and rejects the ids Google cannot resolve', () => {
|
||||
expect(isGooglePlaceId('ChIJLU7jZClu5kcR4PcOOO6p3I0')).toBe(true);
|
||||
expect(isGooglePlaceId('coords:48.8,2.3')).toBe(false);
|
||||
expect(isGooglePlaceId('node:5255005321')).toBe(false);
|
||||
expect(isGooglePlaceId('way:84527326')).toBe(false);
|
||||
expect(isGooglePlaceId('relation:345407')).toBe(false);
|
||||
expect(isGooglePlaceId('https://lh3.googleusercontent.com/photo.jpg')).toBe(false);
|
||||
// The collection views send the bare coordinate pair when a place has no ids.
|
||||
expect(isGooglePlaceId('36.7617499,-3.8448432')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('googleFtidFromMapsUrl', () => {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
/**
|
||||
* Unit tests for placePhotoCache — PPC-001 through PPC-010.
|
||||
* Covers the downscale guard in put(), removeIfUnreferenced(), and sweepOrphans().
|
||||
* Unit tests for placePhotoCache — PPC-001 through PPC-012.
|
||||
* Covers the downscale guard in put(), removeIfUnreferenced(), sweepOrphans(),
|
||||
* and the two negative-cache windows.
|
||||
* Uses a real in-memory SQLite DB and a throwaway temp upload dir
|
||||
* (TREK_PLACE_PHOTO_DIR) so the real uploads tree is never touched.
|
||||
*/
|
||||
@@ -156,3 +157,58 @@ describe('placePhotoCache.sweepOrphans()', () => {
|
||||
expect(fs.existsSync(filePathFor('ref-a'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('placePhotoCache negative cache', () => {
|
||||
function ageError(placeId: string, ms: number): void {
|
||||
testDb
|
||||
.prepare('UPDATE google_place_photo_meta SET error_at = ? WHERE place_id = ?')
|
||||
.run(Date.now() - ms, placeId);
|
||||
}
|
||||
|
||||
it('PPC-009: a place with no photo stays remembered well past the old five-minute window', () => {
|
||||
cache.markError('photo-less');
|
||||
|
||||
ageError('photo-less', 30 * 60 * 1000);
|
||||
expect(cache.getErrored('photo-less')).toBe(true);
|
||||
|
||||
ageError('photo-less', 5 * 60 * 60 * 1000);
|
||||
expect(cache.getErrored('photo-less')).toBe(true);
|
||||
});
|
||||
|
||||
it('PPC-010: the miss expires once it is a day old', () => {
|
||||
cache.markError('stale-miss');
|
||||
ageError('stale-miss', 24 * 60 * 60 * 1000);
|
||||
|
||||
expect(cache.getErrored('stale-miss')).toBe(false);
|
||||
});
|
||||
|
||||
// A failed provider call says nothing about the place, so it must not inherit the
|
||||
// long window a real "this place has no photo" answer gets.
|
||||
it('PPC-011: a failed provider call is forgotten after minutes and never persisted', () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
cache.markError('flaky', 'provider-error');
|
||||
expect(cache.getErrored('flaky')).toBe(true);
|
||||
// Nothing on disk — a restart retries instead of inheriting someone's outage.
|
||||
expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('flaky')).toBeUndefined();
|
||||
|
||||
vi.advanceTimersByTime(5 * 60 * 1000);
|
||||
expect(cache.getErrored('flaky')).toBe(false);
|
||||
|
||||
// The long window belongs to the other case: same age, still remembered.
|
||||
cache.markError('photo-less-too');
|
||||
ageError('photo-less-too', 5 * 60 * 1000);
|
||||
expect(cache.getErrored('photo-less-too')).toBe(true);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('PPC-012: a cached photo clears an earlier failed attempt', async () => {
|
||||
cache.markError('recovered', 'provider-error');
|
||||
expect(cache.getErrored('recovered')).toBe(true);
|
||||
|
||||
await cache.put('recovered', await makeJpeg(40, 40), null);
|
||||
expect(cache.getErrored('recovered')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user