From 40bcc5c67ac96c9bbcc817e457b8f467bd9adead Mon Sep 17 00:00:00 2001 From: Robert Stoll Date: Mon, 20 Dec 2021 11:13:11 +0100 Subject: [PATCH] #150 remove controller.reprocessFountainAtCoords typecheck city moreover: - rename generateLocationData to generateCityData --- config/fountain.properties.ts | 4 +- config/locations.ts | 14 +- server/api/controllers/controller.ts | 149 ++++-------------- server/api/services/database.service.ts | 24 +-- .../services/generateLocationData.service.ts | 10 +- server/api/services/processing.service.ts | 1 + server/common/build.info.ts | 6 +- server/common/shared-constants.ts | 3 +- 8 files changed, 67 insertions(+), 144 deletions(-) diff --git a/config/fountain.properties.ts b/config/fountain.properties.ts index 98a696f3..2983ce63 100644 --- a/config/fountain.properties.ts +++ b/config/fountain.properties.ts @@ -11,7 +11,7 @@ */ import { PropStatus, PROP_STATUS_OK, PROP_STATUS_WARNING } from '../server/common/constants'; -import { locations, mapLocations } from './locations'; +import { locationsCollection, mapLocations } from './locations'; import { isBlacklisted } from '../server/api/services/categories.wm'; import l from '../server/common/logger'; import _ from 'lodash'; @@ -735,7 +735,7 @@ const fountain_properties: FountainPropertiesMeta = { // loop through all catalog codes to find the right one for (const code of catCodes) { // return value only if qualifier matches the operator id - if (_.map(locations, 'operator_fountain_catalog_qid').indexOf(code.qualifiers['P972'][0]) >= 0) { + if (_.map(locationsCollection, 'operator_fountain_catalog_qid').indexOf(code.qualifiers['P972'][0]) >= 0) { return code.value; } } diff --git a/config/locations.ts b/config/locations.ts index 8092b8ab..58cebfdd 100644 --- a/config/locations.ts +++ b/config/locations.ts @@ -513,15 +513,23 @@ Therefore responsible citizens have mapped out all fountains of the festival hos //TODO we could use this type instead of string export type City = keyof typeof internalLocationsCollection; +export const cities: City[] = Object.keys(internalLocationsCollection).filter( + city => city !== 'default' && (city !== 'test' || process.env.NODE_ENV !== 'production') +) as City[]; + +export function isCity(s: string): s is City { + return cities.includes(s as City); +} + export type LocationsCollection = Record; // we don't expose just the internal structure as we also want to be sure that it follows the spec. // However, we allow City union to grow dynamically -export const locations: LocationsCollection = internalLocationsCollection; +export const locationsCollection: LocationsCollection = internalLocationsCollection; export function mapLocations(f: (loc: Location) => R): R[] { const arr: R[] = []; - for (const loc in locations) { - const l = locations[loc]; + for (const loc in locationsCollection) { + const l = locationsCollection[loc]; arr.push(f(l)); } return arr; diff --git a/server/api/controllers/controller.ts b/server/api/controllers/controller.ts index b9721018..cb866b9a 100644 --- a/server/api/controllers/controller.ts +++ b/server/api/controllers/controller.ts @@ -7,8 +7,8 @@ import WikidataService from '../services/wikidata.service'; import l from '../../common/logger'; -import generateLocationData from '../services/generateLocationData.service'; -import { locations } from '../../../config/locations'; +import generateCityData from '../services/generateLocationData.service'; +import { cities, City, isCity, locationsCollection } from '../../../config/locations'; import { fountain_property_metadata } from '../../../config/fountain.properties'; import NodeCache from 'node-cache'; import { essenceOf, fillWikipediaSummary } from '../services/processing.service'; @@ -26,6 +26,8 @@ import { getSingleBooleanQueryParam, getSingleStringQueryParam } from './utils'; import { Fountain, FountainCollection, GalleryValue, isDatabase } from '../../common/typealias'; import { hasWikiCommonsCategories } from '../../common/wikimedia-types'; import { ImageLike } from '../../../config/text2img'; +import applyImpliedPropertiesOsm from '../services/applyImplied.service'; +import { intersectionBy } from 'lodash'; // Configuration of Cache after https://www.npmjs.com/package/node-cache const cityCache = new NodeCache({ @@ -47,7 +49,7 @@ cityCache.on('expired', key => { // check if cache item key is neither the summary nor the list of errors. These will be updated automatically when the detailed city data are updated. if (!key.includes('_essential') && !key.includes('_errors')) { l.info(`controller.js cityCache.on('expired',...): Automatic cache refresh of ${key}`); - generateLocationDataAndCache(key, cityCache); + generateCityDataAndAddToCache(key, cityCache); } }); @@ -55,20 +57,16 @@ export class Controller { constructor() { // In production mode, process all fountains when starting the server so that the data are ready for the first requests if (process.env.NODE_ENV === 'production') { - Object.keys(locations).forEach(locationCode => { - l.info(`controller.js Generating data for ${locationCode}`); - generateLocationData(locationCode).then(fountainCollection => { + cities.forEach(city => { + l.info(`controller.js Generating data for ${city}`); + generateCityData(city).then(fountainCollection => { // save new data to storage //TODO @ralfhauser, the old comment states // expire after two hours but CACHE_FOR_HRS_i45db is currently 48, which means after two days - cityCache.set( - locationCode, - fountainCollection, - 60 * 60 * sharedConstants.CACHE_FOR_HRS_i45db - ); + cityCache.set(city, fountainCollection, 60 * 60 * sharedConstants.CACHE_FOR_HRS_i45db); // create a reduced version of the data as well - cityCache.set(locationCode + '_essential', essenceOf(fountainCollection)); + cityCache.set(city + '_essential', essenceOf(fountainCollection)); // also create list of processing errors (for proximap#206) - cityCache.set(locationCode + '_errors', extractProcessingErrors(fountainCollection)); + cityCache.set(city + '_errors', extractProcessingErrors(fountainCollection)); }); }); } @@ -92,12 +90,15 @@ export class Controller { byLocation(req: Request, res: Response): void { const start = new Date(); const city = getSingleStringQueryParam(req, 'city'); + if (!isCity(city)) { + throw Error('unsupported city given: ' + city); + } const refresh = getSingleBooleanQueryParam(req, 'refresh', /* isOptional = */ true); - // if a refresh is requested or if no data is in the cache, then reprocesses the fountains + // if a refresh is requested or if no data is in the cache, then reprocess the fountains if (refresh || cityCache.keys().indexOf(city) === -1) { l.info(`controller.js byLocation: refresh: ${refresh} , city: ` + city); - generateLocationData(city) + generateCityData(city) .then(fountainCollection => { // save new data to storage cityCache.set(city, fountainCollection, 60 * 60 * sharedConstants.CACHE_FOR_HRS_i45db); @@ -156,7 +157,7 @@ export class Controller { */ getLocationMetadata(_req: Request, res: Response): void { // let gak = locations.gak; - sendJson(res, locations, 'getLocationMetadata'); //res.json(locations); + sendJson(res, locationsCollection, 'getLocationMetadata'); //res.json(locations); l.info('controller.js: getLocationMetadata sent'); } @@ -225,6 +226,9 @@ function sendJson(resp: Response, obj: Record | undefined, dbg: str */ function byId(req: Request, res: Response, forceRefresh: boolean): Promise { const city = getSingleStringQueryParam(req, 'city'); + if (!isCity(city)) { + return new Promise((_, reject) => reject('unsupported city given: ' + city)); + } const database = getSingleStringQueryParam(req, 'database'); if (!isDatabase(database)) { return new Promise((_, reject) => reject('unsupported database given: ' + database)); @@ -240,7 +244,7 @@ function byId(req: Request, res: Response, forceRefresh: boolean): Promise[] = []; if (forceRefresh || fountainCollection === undefined) { l.info('controller.js byId: ' + city + ' not found in cache ' + dbg + ' - start city lazy load'); - const genLocPrms = generateLocationDataAndCache(city, cityCache); + const genLocPrms = generateCityDataAndAddToCache(city, cityCache); cityPromises.push(genLocPrms); } return Promise.all(cityPromises) @@ -525,112 +529,21 @@ function byId(req: Request, res: Response, forceRefresh: boolean): Promise applyImpliedPropertiesOsm(r)) -// .catch(e => { -// l.error(`controller.js reprocessFountainAtCoords: Error collecting OSM data: ${JSON.stringify(e)} `); -// // TODO @ralfhauser, this is an ugly side effect, this does nost stop the program but implies return void -// // hence I changed it because we already catch errors in Promise.all -// // res.status(500).send(e.stack); -// throw e; -// }); - -// const wikidataPromise = WikidataService -// // Fetch all wikidata items within radius -// .idsByCenter(lat, lng, radius, dbg) -// // Fetch detailed information for fountains based on wikidata ids -// .then(r => WikidataService.byIds(r, dbg)) -// .catch(e => { -// l.error(`Error collecting Wikidata data: ${e}`); -// // TODO @ralfhauser, same same as above -// // res.status(500).send(e.stack); -// throw e; -// }); -// const debugAll = true; -// // When both OSM and Wikidata data have been collected, continue with joint processing -// Promise.all([osmPromise, wikidataPromise]) - -// // Get any missing wikidata fountains for #212 (fountains not fetched from Wikidata because not listed as fountains, but referenced by fountains of OSM) -// .then(r => fillInMissingWikidataFountains(r[0], r[1], dbg)) - -// // Conflate osm and wikidata fountains together -// .then(r => -// conflate( -// { -// osm: r.osm, -// wikidata: r.wikidata, -// }, -// dbg, -// debugAll -// ) -// ) - -// // return only the fountain that is closest to the coordinates of the query -// .then(r => { -// const distances = _.map(r, f => { -// // compute distance to center for each fountain -// return haversine(f.geometry.coordinates, [lng, lat], { -// unit: 'meter', -// format: '[lon,lat]', -// }); -// }); -// // return closest -// const closest = r[_.indexOf(distances, _.min(distances))]; -// return [closest]; -// }) - -// // fetch more information about fountains (Artist information, gallery, etc.) -// //TOOD @ralfhauser, the last parameter for debugAll was missing undefined is falsy hence I used false -// .then(r => defaultCollectionEnhancement(r, dbg, false)) - -// // Update cache with newly processed fountain -// .then(r => { -// const city = getSingleStringQueryParam(req, 'city'); -// const closest = updateCacheWithFountain(cityCache, r[0], city); -// sendJson(res, closest, 'after updateCacheWithFountain'); -// }) -// .catch(e => { -// l.error(`Error collecting data: ${e.stack}`); -// res.status(500).send(e.stack); -// }); -// } - -export function generateLocationDataAndCache(key: string, cityCache: NodeCache): Promise { +export function generateCityDataAndAddToCache(city: City, cityCache: NodeCache): Promise { // trigger a reprocessing of the location's data, based on the key. - const genLocPrms = generateLocationData(key) + const genLocPrms = generateCityData(city) .then(fountainCollection => { // save newly generated fountainCollection to the cache - let ftns = -1; - if (null != fountainCollection && null != fountainCollection.features) { - ftns = fountainCollection.features.length; + let numberOfFountains = -1; + if (fountainCollection?.features != null) { + numberOfFountains = fountainCollection.features.length; } //TODO @ralfhauser, the old comment states // expire after two hours but CACHE_FOR_HRS_i45db is currently 48, which means after two days - cityCache.set(key, fountainCollection, 60 * 60 * sharedConstants.CACHE_FOR_HRS_i45db); // expire after two hours + cityCache.set(city, fountainCollection, 60 * 60 * sharedConstants.CACHE_FOR_HRS_i45db); // expire after two hours // create a reduced version of the data as well const essence = essenceOf(fountainCollection); - cityCache.set(key + '_essential', essence); + cityCache.set(city + '_essential', essence); let ess = -1; if (null != essence && null != essence.features) { ess = essence.features.length; @@ -638,7 +551,7 @@ export function generateLocationDataAndCache(key: string, cityCache: NodeCache): // also create list of processing errors (for proximap#206) const processingErrors = extractProcessingErrors(fountainCollection); - cityCache.set(key + '_errors', processingErrors); + cityCache.set(city + '_errors', processingErrors); //TODO @ralfhauser, processingErrors is never null but an array, which also means processingErrors.features never exists and hence this will always be false // let prcErr = -1; // if (null != processingErrors && null != processingErrors.features) { @@ -646,9 +559,9 @@ export function generateLocationDataAndCache(key: string, cityCache: NodeCache): // } const prcErr = processingErrors.length; l.info( - `generateLocationDataAndCache setting cache of ${key} ` + + `generateLocationDataAndCache setting cache of ${city} ` + ' ftns: ' + - ftns + + numberOfFountains + ' ess: ' + ess + ' prcErr: ' + diff --git a/server/api/services/database.service.ts b/server/api/services/database.service.ts index 553e3080..7657683a 100644 --- a/server/api/services/database.service.ts +++ b/server/api/services/database.service.ts @@ -10,40 +10,42 @@ import { essenceOf } from './processing.service'; // import {CACHE_FOR_HRS_i45db} from "../../common/constants"; const CACHE_FOR_HRS_i45db = 1; import l from '../../common/logger'; -import { generateLocationDataAndCache } from '../controllers/controller'; +import { generateCityDataAndAddToCache } from '../controllers/controller'; import _ from 'lodash'; import haversine from 'haversine'; import NodeCache from 'node-cache'; import {} from 'geojson'; import { Fountain, FountainCollection } from '../../common/typealias'; +import { City } from '../../../config/locations'; -export function updateCacheWithFountain(cache: NodeCache, fountain: Fountain, cityName: string): Fountain { +export function updateCacheWithFountain(cache: NodeCache, fountain: Fountain, city: City): Fountain { // updates cache and returns fountain with datablue id // get city data from cache - let fountains = cache.get(cityName); + let fountains = cache.get(city); const cacheTimeInSecs = 60 * 60 * CACHE_FOR_HRS_i45db; - if (!fountains && !cityName.includes('_essential') && !cityName.includes('_errors')) { + if (!fountains && !city.includes('_essential') && !city.includes('_errors')) { l.info( - `updateCacheWithFountain server-side city data disappeared (server restart?) - cache recreation for ${cityName}` + `updateCacheWithFountain server-side city data disappeared (server restart?) - cache recreation for ${city}` ); - generateLocationDataAndCache(cityName, cache); - fountains = cache.get(cityName); + generateCityDataAndAddToCache(city, cache); + //TODO @ralf.hauser, this is buggy as generateCityDataAndAddToCache returns a promise and most likely did not finish at this point + fountains = cache.get(city); } if (fountains) { // replace fountain - [fountains, fountain] = replaceFountain(fountains, fountain, cityName); + [fountains, fountain] = replaceFountain(fountains, fountain, city); // send to cache //TODO consider whether really to fully extend the cache-time for the whole city just because one fountain was refreshed // a remaining city-cache-time could be calculated with getTtl(cityname) - cache.set(cityName, fountains, cacheTimeInSecs); + cache.set(city, fountains, cacheTimeInSecs); // create a reduced version of the data as well const r_essential = essenceOf(fountains); - cache.set(cityName + '_essential', r_essential, cacheTimeInSecs); + cache.set(city + '_essential', r_essential, cacheTimeInSecs); return fountain; } l.info( 'database.services.js updateCacheWithFountain: no fountains were in cache of city ' + - cityName + + city + ' tried to work on ' + fountain ); diff --git a/server/api/services/generateLocationData.service.ts b/server/api/services/generateLocationData.service.ts index e7fc590f..bb84e4b8 100644 --- a/server/api/services/generateLocationData.service.ts +++ b/server/api/services/generateLocationData.service.ts @@ -6,7 +6,7 @@ */ import l from '../../common/logger'; -import { locations } from '../../../config/locations'; +import { City, locationsCollection } from '../../../config/locations'; import OsmService from '../services/osm.service'; import WikidataService from '../services/wikidata.service'; import { conflate } from '../services/conflate.data.service'; @@ -23,7 +23,7 @@ import { MediaWikiSimplifiedEntity } from '../../common/wikimedia-types'; * This function creates fountain collections * @param {string} locationName - the code name of the location for which fountains should be processed */ -function generateLocationData(locationName: string): Promise { +function generateCityData(locationName: City): Promise { const start = new Date(); l.info(`generateLocationData.service.js: processing all fountains from "${locationName}" `); @@ -34,9 +34,7 @@ function generateLocationData(locationName: string): Promise }; // get bounding box of location - const location = Object.prototype.hasOwnProperty.call(locations, locationName) - ? locations[locationName] - : undefined; + const location = locationsCollection[locationName]; if (location == undefined) { logAndRejectError(`location not found in config: ${locationName}`); } else { @@ -145,4 +143,4 @@ function generateLocationData(locationName: string): Promise }); } -export default generateLocationData; +export default generateCityData; diff --git a/server/api/services/processing.service.ts b/server/api/services/processing.service.ts index a6c2fb6f..2c4f7399 100644 --- a/server/api/services/processing.service.ts +++ b/server/api/services/processing.service.ts @@ -167,6 +167,7 @@ export function fillWikipediaSummaries(fountainArr: Fountain[], dbg: string): Pr export function createUniqueIds(fountainArr: Fountain[]): Promise { // takes a collection of fountains and returns the same collection, enhanced with unique and as persistent as possible ids + //TODO @ralf.hauser I don't know what this id is used for but this implementation does definitely not generate unique ids return new Promise(resolve => { let i_n = 0; fountainArr.forEach(f => { diff --git a/server/common/build.info.ts b/server/common/build.info.ts index d07f72ba..b07f204a 100644 --- a/server/common/build.info.ts +++ b/server/common/build.info.ts @@ -1,9 +1,9 @@ // this file is automatically generated by git.version.js script const buildInfo = { version: '', - revision: '88376dc', + revision: '7acb2bc', branch: '#150-forceRefresh-id', - commit_time: '2021-12-20 09:34:27 +0100', - build_time: 'Mon Dec 20 2021 10:30:17 GMT+0100 (Central European Standard Time)', + commit_time: '2021-12-20 10:31:35 +0100', + build_time: 'Mon Dec 20 2021 11:11:32 GMT+0100 (Central European Standard Time)', }; export default buildInfo; diff --git a/server/common/shared-constants.ts b/server/common/shared-constants.ts index 45edbde6..a88c9ec7 100644 --- a/server/common/shared-constants.ts +++ b/server/common/shared-constants.ts @@ -12,7 +12,8 @@ * */ export default { - LANGS: ['en', 'de', 'fr', 'it', 'tr', 'sr'], + //TODO proximap#394 reactivate serbian again, see also TODO in proximap + LANGS: ['en', 'de', 'fr', 'it', 'tr' /* 'sr' */], CACHE_FOR_HRS_i45db: 48, gak: `${process.env.GOOGLE_API_KEY}`, };