elgaard/MapComplete
1
0
Fork 0
forked from MapComplete/MapComplete
Code Pull requests Activity

Better error handling

Browse source
This commit is contained in:
Pieter Vander Vennet 2022-04-22 03:17:40 +02:00
parent e5dbeee621
commit ad3a776366
6 changed files with 106 additions and 78 deletions
Download patch file Download diff file Expand all files Collapse all files

19
Models/ThemeConfig/Conversion/Conversion.ts
View file

@ -202,13 +202,18 @@ export class Fuse<T> extends DesugaringStep<T> {
const information = [] const information = []
for (let i = 0; i < this.steps.length; i++) { for (let i = 0; i < this.steps.length; i++) {
const step = this.steps[i]; const step = this.steps[i];
let r = step.convert(json, "While running step " + step.name + ": " + context) try{
errors.push(...r.errors ?? []) let r = step.convert(json, "While running step " + step.name + ": " + context)
warnings.push(...r.warnings ?? []) errors.push(...r.errors ?? [])
information.push(...r.information ?? []) warnings.push(...r.warnings ?? [])
json = r.result information.push(...r.information ?? [])
if (errors.length > 0) { json = r.result
break; if (errors.length > 0) {
break;
}
}catch(e){
console.error("Step "+step.name+" failed due to "+e);
throw e
} }
} }
return { return {

88
Models/ThemeConfig/Conversion/PrepareTheme.ts
View file

@ -13,23 +13,25 @@ import {AddContextToTranslations} from "./AddContextToTranslations";
class SubstituteLayer extends Conversion<(string | LayerConfigJson), LayerConfigJson[]> { class SubstituteLayer extends Conversion<(string | LayerConfigJson), LayerConfigJson[]> {
private readonly _state: DesugaringContext; private readonly _state: DesugaringContext;
constructor( constructor(
state: DesugaringContext, state: DesugaringContext,
) { ) {
super("Converts the identifier of a builtin layer into the actual layer, or converts a 'builtin' syntax with override in the fully expanded form", [],"SubstituteLayer"); super("Converts the identifier of a builtin layer into the actual layer, or converts a 'builtin' syntax with override in the fully expanded form", [], "SubstituteLayer");
this._state = state; this._state = state;
} }
convert(json: string | LayerConfigJson, context: string): { result: LayerConfigJson[]; errors: string[], information?: string[] } { convert(json: string | LayerConfigJson, context: string): { result: LayerConfigJson[]; errors: string[], information?: string[] } {
const errors = [] const errors = []
const information = [] const information = []
const state= this._state const state = this._state
function reportNotFound(name: string){
function reportNotFound(name: string) {
const knownLayers = Array.from(state.sharedLayers.keys()) const knownLayers = Array.from(state.sharedLayers.keys())
const withDistance = knownLayers.map(lname => [lname, Utils.levenshteinDistance(name, lname)]) const withDistance = knownLayers.map(lname => [lname, Utils.levenshteinDistance(name, lname)])
withDistance.sort((a, b) => a[1] - b[1]) withDistance.sort((a, b) => a[1] - b[1])
const ids = withDistance.map(n => n[0]) const ids = withDistance.map(n => n[0])
// Known builtin layers are "+.join(",")+"\n For more information, see " // Known builtin layers are "+.join(",")+"\n For more information, see "
errors.push(`${context}: The layer with name ${name} was not found as a builtin layer. Perhaps you meant ${ids[0]}, ${ids[1]} or ${ids[2]}? errors.push(`${context}: The layer with name ${name} was not found as a builtin layer. Perhaps you meant ${ids[0]}, ${ids[1]} or ${ids[2]}?
For an overview of all available layers, refer to https://github.com/pietervdvn/MapComplete/blob/develop/Docs/BuiltinLayers.md`) For an overview of all available layers, refer to https://github.com/pietervdvn/MapComplete/blob/develop/Docs/BuiltinLayers.md`)
} }
@ -73,39 +75,39 @@ class SubstituteLayer extends Conversion<(string | LayerConfigJson), LayerConfig
errors.push(`At ${context}: could not apply an override due to: ${e}.\nThe override is: ${JSON.stringify(json["override"],)}`) errors.push(`At ${context}: could not apply an override due to: ${e}.\nThe override is: ${JSON.stringify(json["override"],)}`)
} }
if(json["hideTagRenderingsWithLabels"]){ if (json["hideTagRenderingsWithLabels"]) {
const hideLabels: Set<string> = new Set(json["hideTagRenderingsWithLabels"]) const hideLabels: Set<string> = new Set(json["hideTagRenderingsWithLabels"])
// These labels caused at least one deletion // These labels caused at least one deletion
const usedLabels : Set<string> = new Set<string>(); const usedLabels: Set<string> = new Set<string>();
const filtered = [] const filtered = []
for (const tr of found.tagRenderings) { for (const tr of found.tagRenderings) {
const labels = tr["labels"] const labels = tr["labels"]
if(labels !== undefined){ if (labels !== undefined) {
const forbiddenLabel = labels.findIndex(l => hideLabels.has(l)) const forbiddenLabel = labels.findIndex(l => hideLabels.has(l))
if(forbiddenLabel >= 0){ if (forbiddenLabel >= 0) {
usedLabels.add(labels[forbiddenLabel]) usedLabels.add(labels[forbiddenLabel])
information.push(context+": Dropping tagRendering "+tr["id"]+" as it has a forbidden label: "+labels[forbiddenLabel]) information.push(context + ": Dropping tagRendering " + tr["id"] + " as it has a forbidden label: " + labels[forbiddenLabel])
continue continue
} }
} }
if(hideLabels.has(tr["id"])){ if (hideLabels.has(tr["id"])) {
usedLabels.add(tr["id"]) usedLabels.add(tr["id"])
information.push(context+": Dropping tagRendering "+tr["id"]+" as its id is a forbidden label") information.push(context + ": Dropping tagRendering " + tr["id"] + " as its id is a forbidden label")
continue continue
} }
if(hideLabels.has(tr["group"])){ if (hideLabels.has(tr["group"])) {
usedLabels.add(tr["group"]) usedLabels.add(tr["group"])
information.push(context+": Dropping tagRendering "+tr["id"]+" as its group `"+tr["group"]+"` is a forbidden label") information.push(context + ": Dropping tagRendering " + tr["id"] + " as its group `" + tr["group"] + "` is a forbidden label")
continue continue
} }
filtered.push(tr) filtered.push(tr)
} }
const unused = Array.from(hideLabels).filter(l => !usedLabels.has(l)) const unused = Array.from(hideLabels).filter(l => !usedLabels.has(l))
if(unused.length > 0){ if (unused.length > 0) {
errors.push("This theme specifies that certain tagrenderings have to be removed based on forbidden layers. One or more of these layers did not match any tagRenderings and caused no deletions: "+unused.join(", ")+"\n This means that this label can be removed or that the original tagRendering that should be deleted does not have this label anymore") errors.push("This theme specifies that certain tagrenderings have to be removed based on forbidden layers. One or more of these layers did not match any tagRenderings and caused no deletions: " + unused.join(", ") + "\n This means that this label can be removed or that the original tagRendering that should be deleted does not have this label anymore")
} }
found.tagRenderings = filtered found.tagRenderings = filtered
} }
@ -130,7 +132,7 @@ class AddDefaultLayers extends DesugaringStep<LayoutConfigJson> {
private _state: DesugaringContext; private _state: DesugaringContext;
constructor(state: DesugaringContext) { constructor(state: DesugaringContext) {
super("Adds the default layers, namely: " + Constants.added_by_default.join(", "), ["layers"],"AddDefaultLayers"); super("Adds the default layers, namely: " + Constants.added_by_default.join(", "), ["layers"], "AddDefaultLayers");
this._state = state; this._state = state;
} }
@ -147,8 +149,8 @@ class AddDefaultLayers extends DesugaringStep<LayoutConfigJson> {
errors.push("Default layer " + layerName + " not found") errors.push("Default layer " + layerName + " not found")
continue continue
} }
if(alreadyLoaded.has(v.id)){ if (alreadyLoaded.has(v.id)) {
warnings.push("Layout "+context+" already has a layer with name "+v.id+"; skipping inclusion of this builtin layer") warnings.push("Layout " + context + " already has a layer with name " + v.id + "; skipping inclusion of this builtin layer")
continue continue
} }
json.layers.push(v) json.layers.push(v)
@ -165,7 +167,7 @@ class AddDefaultLayers extends DesugaringStep<LayoutConfigJson> {
class AddImportLayers extends DesugaringStep<LayoutConfigJson> { class AddImportLayers extends DesugaringStep<LayoutConfigJson> {
constructor() { constructor() {
super("For every layer in the 'layers'-list, create a new layer which'll import notes. (Note that priviliged layers and layers which have a geojson-source set are ignored)", ["layers"],"AddImportLayers"); super("For every layer in the 'layers'-list, create a new layer which'll import notes. (Note that priviliged layers and layers which have a geojson-source set are ignored)", ["layers"], "AddImportLayers");
} }
convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors: string[] } { convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors: string[] } {
@ -176,7 +178,7 @@ class AddImportLayers extends DesugaringStep<LayoutConfigJson> {
json.layers = [...json.layers] json.layers = [...json.layers]
if(json.enableNoteImports ?? true) { if (json.enableNoteImports ?? true) {
const creator = new CreateNoteImportLayer() const creator = new CreateNoteImportLayer()
for (let i1 = 0; i1 < allLayers.length; i1++) { for (let i1 = 0; i1 < allLayers.length; i1++) {
const layer = allLayers[i1]; const layer = allLayers[i1];
@ -222,8 +224,9 @@ class AddImportLayers extends DesugaringStep<LayoutConfigJson> {
export class AddMiniMap extends DesugaringStep<LayerConfigJson> { export class AddMiniMap extends DesugaringStep<LayerConfigJson> {
private readonly _state: DesugaringContext; private readonly _state: DesugaringContext;
constructor(state: DesugaringContext, ) {
super("Adds a default 'minimap'-element to the tagrenderings if none of the elements define such a minimap", ["tagRenderings"],"AddMiniMap"); constructor(state: DesugaringContext,) {
super("Adds a default 'minimap'-element to the tagrenderings if none of the elements define such a minimap", ["tagRenderings"], "AddMiniMap");
this._state = state; this._state = state;
} }
@ -280,10 +283,10 @@ export class AddMiniMap extends DesugaringStep<LayerConfigJson> {
} }
} }
class AddContextToTransltionsInLayout extends DesugaringStep <LayoutConfigJson>{ class AddContextToTransltionsInLayout extends DesugaringStep <LayoutConfigJson> {
constructor() { constructor() {
super("Adds context to translations, including the prefix 'themes:json.id'; this is to make sure terms in an 'overrides' or inline layer are linkable too",["_context"], "AddContextToTranlationsInLayout"); super("Adds context to translations, including the prefix 'themes:json.id'; this is to make sure terms in an 'overrides' or inline layer are linkable too", ["_context"], "AddContextToTranlationsInLayout");
} }
convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors?: string[]; warnings?: string[]; information?: string[] } { convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors?: string[]; warnings?: string[]; information?: string[] } {
@ -296,7 +299,7 @@ class AddContextToTransltionsInLayout extends DesugaringStep <LayoutConfigJson>{
class ApplyOverrideAll extends DesugaringStep<LayoutConfigJson> { class ApplyOverrideAll extends DesugaringStep<LayoutConfigJson> {
constructor() { constructor() {
super("Applies 'overrideAll' onto every 'layer'. The 'overrideAll'-field is removed afterwards", ["overrideAll", "layers"],"ApplyOverrideAll"); super("Applies 'overrideAll' onto every 'layer'. The 'overrideAll'-field is removed afterwards", ["overrideAll", "layers"], "ApplyOverrideAll");
} }
convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors: string[]; warnings: string[] } { convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors: string[]; warnings: string[] } {
@ -325,8 +328,9 @@ class ApplyOverrideAll extends DesugaringStep<LayoutConfigJson> {
class AddDependencyLayersToTheme extends DesugaringStep<LayoutConfigJson> { class AddDependencyLayersToTheme extends DesugaringStep<LayoutConfigJson> {
private readonly _state: DesugaringContext; private readonly _state: DesugaringContext;
constructor(state: DesugaringContext, ) {
super("If a layer has a dependency on another layer, these layers are added automatically on the theme. (For example: defibrillator depends on 'walls_and_buildings' to snap onto. This layer is added automatically)", ["layers"],"AddDependencyLayersToTheme"); constructor(state: DesugaringContext,) {
super("If a layer has a dependency on another layer, these layers are added automatically on the theme. (For example: defibrillator depends on 'walls_and_buildings' to snap onto. This layer is added automatically)", ["layers"], "AddDependencyLayersToTheme");
this._state = state; this._state = state;
} }
@ -340,17 +344,17 @@ class AddDependencyLayersToTheme extends DesugaringStep<LayoutConfigJson> {
const dependencies: { neededLayer: string, reason: string, context?: string, neededBy: string }[] = [] const dependencies: { neededLayer: string, reason: string, context?: string, neededBy: string }[] = []
for (const layerConfig of alreadyLoaded) { for (const layerConfig of alreadyLoaded) {
try{ try {
const layerDeps = DependencyCalculator.getLayerDependencies(new LayerConfig(layerConfig)) const layerDeps = DependencyCalculator.getLayerDependencies(new LayerConfig(layerConfig))
dependencies.push(...layerDeps) dependencies.push(...layerDeps)
}catch(e){ } catch (e) {
console.error(e) console.error(e)
throw "Detecting layer dependencies for "+layerConfig.id+" failed due to "+e throw "Detecting layer dependencies for " + layerConfig.id + " failed due to " + e
} }
} }
for (const dependency of dependencies) { for (const dependency of dependencies) {
if(loadedLayerIds.has(dependency.neededLayer)){ if (loadedLayerIds.has(dependency.neededLayer)) {
// We mark the needed layer as 'mustLoad' // We mark the needed layer as 'mustLoad'
alreadyLoaded.find(l => l.id === dependency.neededLayer).forceLoad = true alreadyLoaded.find(l => l.id === dependency.neededLayer).forceLoad = true
} }
@ -418,13 +422,14 @@ class AddDependencyLayersToTheme extends DesugaringStep<LayoutConfigJson> {
class PreparePersonalTheme extends DesugaringStep<LayoutConfigJson> { class PreparePersonalTheme extends DesugaringStep<LayoutConfigJson> {
private readonly _state: DesugaringContext; private readonly _state: DesugaringContext;
constructor(state: DesugaringContext) { constructor(state: DesugaringContext) {
super("Adds every public layer to the personal theme",["layers"],"PreparePersonalTheme"); super("Adds every public layer to the personal theme", ["layers"], "PreparePersonalTheme");
this._state = state; this._state = state;
} }
convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors?: string[]; warnings?: string[]; information?: string[] } { convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors?: string[]; warnings?: string[]; information?: string[] } {
if(json.id !== "personal"){ if (json.id !== "personal") {
return {result: json} return {result: json}
} }
@ -434,30 +439,30 @@ class PreparePersonalTheme extends DesugaringStep<LayoutConfigJson> {
} }
class WarnForUnsubstitutedLayersInTheme extends DesugaringStep<LayoutConfigJson>{ class WarnForUnsubstitutedLayersInTheme extends DesugaringStep<LayoutConfigJson> {
constructor() { constructor() {
super("Generates a warning if a theme uses an unsubstituted layer", ["layers"],"WarnForUnsubstitutedLayersInTheme"); super("Generates a warning if a theme uses an unsubstituted layer", ["layers"], "WarnForUnsubstitutedLayersInTheme");
} }
convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors?: string[]; warnings?: string[]; information?: string[] } { convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors?: string[]; warnings?: string[]; information?: string[] } {
if(json.hideFromOverview === true){ if (json.hideFromOverview === true) {
return {result: json} return {result: json}
} }
const warnings = [] const warnings = []
for (const layer of json.layers) { for (const layer of json.layers) {
if(typeof layer === "string"){ if (typeof layer === "string") {
continue continue
} }
if(layer["builtin"] !== undefined){ if (layer["builtin"] !== undefined) {
continue continue
} }
if(layer["source"]["geojson"] !== undefined){ if (layer["source"]["geojson"] !== undefined) {
// We turn a blind eye for import layers // We turn a blind eye for import layers
continue continue
} }
const wrn = "The theme "+json.id+" has an inline layer: "+layer["id"]+". This is discouraged." const wrn = "The theme " + json.id + " has an inline layer: " + layer["id"] + ". This is discouraged."
warnings.push(wrn) warnings.push(wrn)
} }
return { return {
@ -472,6 +477,7 @@ export class PrepareTheme extends Fuse<LayoutConfigJson> {
constructor(state: DesugaringContext) { constructor(state: DesugaringContext) {
super( super(
"Fully prepares and expands a theme", "Fully prepares and expands a theme",
new AddContextToTransltionsInLayout(), new AddContextToTransltionsInLayout(),
new PreparePersonalTheme(state), new PreparePersonalTheme(state),
new WarnForUnsubstitutedLayersInTheme(), new WarnForUnsubstitutedLayersInTheme(),

11
Models/ThemeConfig/Conversion/Validation.ts
View file

@ -217,12 +217,17 @@ class MiscThemeChecks extends DesugaringStep<LayoutConfigJson>{
convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors?: string[]; warnings?: string[]; information?: string[] } { convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors?: string[]; warnings?: string[]; information?: string[] } {
const warnings = [] const warnings = []
const errors = []
if(json.id !== "personal" && (json.layers === undefined || json.layers.length === 0)){
errors.push("The theme "+json.id+" has no 'layers' defined ("+context+")")
}
if(json.socialImage === ""){ if(json.socialImage === ""){
warnings.push("Social image for theme "+json.id+" is the emtpy string") warnings.push("Social image for theme "+json.id+" is the emtpy string")
} }
return { return {
result :json, result :json,
warnings warnings,
errors
}; };
} }
} }
@ -231,8 +236,8 @@ export class PrevalidateTheme extends Fuse<LayoutConfigJson> {
constructor() { constructor() {
super("Various consistency checks on the raw JSON", super("Various consistency checks on the raw JSON",
new OverrideShadowingCheck(), new MiscThemeChecks(),
new MiscThemeChecks() new OverrideShadowingCheck()
); );
} }

7
Models/ThemeConfig/Json/UnitConfigJson.ts
View file

@ -18,9 +18,12 @@ export default interface UnitConfigJson {
export interface ApplicableUnitJson { export interface ApplicableUnitJson {
/** /**
* The canonical value which will be added to the text. * The canonical value which will be added to the value in OSM.
* e.g. "m" for meters * e.g. "m" for meters
* If the user inputs '42', the canonical value will be added and it'll become '42m' * If the user inputs '42', the canonical value will be added and it'll become '42m'.
*
* Important: often, _no_ canonical values are expected, e.g. in the case of 'maxspeed' where 'km/h' is the default.
* In this case, an empty string should be used
*/ */
canonicalDenomination: string, canonicalDenomination: string,
/** /**

3
Models/ThemeConfig/LayerConfig.ts
View file

@ -133,6 +133,9 @@ export default class LayerConfig extends WithContextLoader {
this.allowSplit = json.allowSplit ?? false; this.allowSplit = json.allowSplit ?? false;
this.name = Translations.T(json.name, translationContext + ".name"); this.name = Translations.T(json.name, translationContext + ".name");
if(json.units!==undefined && !Array.isArray(json.units)){
throw "At "+context+".units: the 'units'-section should be a list; you probably have an object there"
}
this.units = (json.units ?? []).map(((unitJson, i) => Unit.fromJson(unitJson, `${context}.unit[${i}]`))) this.units = (json.units ?? []).map(((unitJson, i) => Unit.fromJson(unitJson, `${context}.unit[${i}]`)))
if (json.description !== undefined) { if (json.description !== undefined) {

22
scripts/generateLayerOverview.ts
View file

@ -203,16 +203,22 @@ class LayerOverviewUtils {
const themePath = themeInfo.path const themePath = themeInfo.path
new PrevalidateTheme().convertStrict(themeFile, themePath) new PrevalidateTheme().convertStrict(themeFile, themePath)
themeFile = new PrepareTheme(convertState).convertStrict(themeFile, themePath) try{
if(knownImagePaths === undefined){ themeFile = new PrepareTheme(convertState).convertStrict(themeFile, themePath)
throw "Could not load known images/licenses"
if(knownImagePaths === undefined){
throw "Could not load known images/licenses"
}
new ValidateThemeAndLayers(knownImagePaths, themePath, true, convertState.tagRenderings)
.convertStrict(themeFile, themePath)
this.writeTheme(themeFile)
fixed.set(themeFile.id, themeFile)
}catch(e){
console.error("ERROR: could not prepare theme "+themePath+" due to "+e)
throw e;
} }
new ValidateThemeAndLayers(knownImagePaths, themePath, true, convertState.tagRenderings)
.convertStrict(themeFile, themePath)
this.writeTheme(themeFile)
fixed.set(themeFile.id, themeFile)
} }
this.writeSmallOverview(themeFiles.map(tf => { this.writeSmallOverview(themeFiles.map(tf => {