diff --git a/Docs/Tools/GenerateSeries.ts b/Docs/Tools/GenerateSeries.ts index 9f135c54d..b97d7399b 100644 --- a/Docs/Tools/GenerateSeries.ts +++ b/Docs/Tools/GenerateSeries.ts @@ -528,7 +528,7 @@ function stackHists(hists: [V, Histogram][]): [V, Histogram][] { runningTotals.bumpHist(hist) result.push([vhist[0], clone]) }) - result.reverse() + result.reverse(/* Changes in place, safe copy*/) return result } diff --git a/Logic/Actors/AvailableBaseLayersImplementation.ts b/Logic/Actors/AvailableBaseLayersImplementation.ts index 2ac7aa8d4..6b12013fe 100644 --- a/Logic/Actors/AvailableBaseLayersImplementation.ts +++ b/Logic/Actors/AvailableBaseLayersImplementation.ts @@ -239,7 +239,7 @@ export default class AvailableBaseLayersImplementation implements AvailableBaseL prefered = preferedCategory.data; } - prefered.reverse(); + prefered.reverse(/*New list, inplace reverse is fine*/); for (const category of prefered) { //Then sort all 'photo'-layers to the top. Stability of the sorting will force a 'best' photo layer on top available.sort((a, b) => { diff --git a/Logic/FeatureSource/FeaturePipeline.ts b/Logic/FeatureSource/FeaturePipeline.ts index bb2382aff..87d4aedda 100644 --- a/Logic/FeatureSource/FeaturePipeline.ts +++ b/Logic/FeatureSource/FeaturePipeline.ts @@ -75,7 +75,7 @@ export default class FeaturePipeline { this.state = state; const self = this - const expiryInSeconds = Math.min(...state.layoutToUse.layers.map(l => l.maxAgeOfCache)) + const expiryInSeconds = Math.min(...state.layoutToUse?.layers?.map(l => l.maxAgeOfCache) ?? []) this.oldestAllowedDate = new Date(new Date().getTime() - expiryInSeconds); this.osmSourceZoomLevel = state.osmApiTileSize.data; const useOsmApi = state.locationControl.map(l => l.zoom > (state.overpassMaxZoom.data ?? 12)) diff --git a/Logic/FeatureSource/Sources/ChangeGeometryApplicator.ts b/Logic/FeatureSource/Sources/ChangeGeometryApplicator.ts index 83ab174bd..d7d6c8b7a 100644 --- a/Logic/FeatureSource/Sources/ChangeGeometryApplicator.ts +++ b/Logic/FeatureSource/Sources/ChangeGeometryApplicator.ts @@ -74,7 +74,7 @@ export default class ChangeGeometryApplicator implements FeatureSourceForLayer { // We only apply the last change as that one'll have the latest geometry const change = changesForFeature[changesForFeature.length - 1] copy.feature.geometry = ChangeDescriptionTools.getGeojsonGeometry(change) - console.log("Applying a geometry change onto ", feature, change, copy) + console.log("Applying a geometry change onto:", feature,"The change is:", change,"which becomes:", copy) newFeatures.push(copy) } this.features.setData(newFeatures) diff --git a/Logic/FeatureSource/TiledFeatureSource/OsmFeatureSource.ts b/Logic/FeatureSource/TiledFeatureSource/OsmFeatureSource.ts index 68190d909..a9bd149b5 100644 --- a/Logic/FeatureSource/TiledFeatureSource/OsmFeatureSource.ts +++ b/Logic/FeatureSource/TiledFeatureSource/OsmFeatureSource.ts @@ -79,7 +79,7 @@ export default class OsmFeatureSource { }) - const neededLayers = options.state.layoutToUse.layers + const neededLayers = (options.state.layoutToUse?.layers ?? []) .filter(layer => !layer.doNotDownload) .filter(layer => layer.source.geojsonSource === undefined || layer.source.isOsmCacheLayer) this.allowedTags = new Or(neededLayers.map(l => l.source.osmTags)) diff --git a/Logic/Osm/Actions/ChangeDescription.ts b/Logic/Osm/Actions/ChangeDescription.ts index 0f03caf0b..42df3f4a0 100644 --- a/Logic/Osm/Actions/ChangeDescription.ts +++ b/Logic/Osm/Actions/ChangeDescription.ts @@ -81,7 +81,7 @@ export class ChangeDescriptionTools { case "way": const w = new OsmWay(change.id) w.nodes = change.changes["nodes"] - w.coordinates = change.changes["coordinates"].map(coor => coor.reverse()) + w.coordinates = change.changes["coordinates"].map(([lon, lat]) => [lat, lon]) return w.asGeoJson().geometry case "relation": const r = new OsmRelation(change.id) diff --git a/Logic/Osm/Actions/CreateMultiPolygonWithPointReuseAction.ts b/Logic/Osm/Actions/CreateMultiPolygonWithPointReuseAction.ts index d5e3f53ec..73eb1f3a1 100644 --- a/Logic/Osm/Actions/CreateMultiPolygonWithPointReuseAction.ts +++ b/Logic/Osm/Actions/CreateMultiPolygonWithPointReuseAction.ts @@ -33,12 +33,12 @@ export default class CreateMultiPolygonWithPointReuseAction extends OsmCreateAct super(null, true); this._tags = [...tags, new Tag("type", "multipolygon")]; this.changeType = changeType; - this.theme = state.layoutToUse.id + this.theme = state?.layoutToUse?.id ?? "" this.createOuterWay = new CreateWayWithPointReuseAction([], outerRingCoordinates, state, config) this.createInnerWays = innerRingsCoordinates.map(ringCoordinates => new CreateNewWayAction([], ringCoordinates.map(([lon, lat]) => ({lat, lon})), - {theme: state.layoutToUse.id})) + {theme: state?.layoutToUse?.id})) this.geojsonPreview = { type: "Feature", diff --git a/Logic/Osm/Actions/CreateNewNodeAction.ts b/Logic/Osm/Actions/CreateNewNodeAction.ts index 1c7d37989..844463ec1 100644 --- a/Logic/Osm/Actions/CreateNewNodeAction.ts +++ b/Logic/Osm/Actions/CreateNewNodeAction.ts @@ -112,16 +112,25 @@ export default class CreateNewNodeAction extends OsmCreateAction { const geojson = this._snapOnto.asGeoJson() const projected = GeoOperations.nearestPoint(geojson, [this._lon, this._lat]) + const projectedCoor= <[number, number]>projected.geometry.coordinates const index = projected.properties.index // We check that it isn't close to an already existing point let reusedPointId = undefined; - const prev = <[number, number]>geojson.geometry.coordinates[index] - if (GeoOperations.distanceBetween(prev, <[number, number]>projected.geometry.coordinates) < this._reusePointDistance) { + let outerring : [number,number][]; + + if(geojson.geometry.type === "LineString"){ + outerring = <[number, number][]> geojson.geometry.coordinates + }else if(geojson.geometry.type === "Polygon"){ + outerring =<[number, number][]> geojson.geometry.coordinates[0] + } + + const prev= outerring[index] + if (GeoOperations.distanceBetween(prev, projectedCoor) < this._reusePointDistance) { // We reuse this point instead! reusedPointId = this._snapOnto.nodes[index] } - const next = <[number, number]>geojson.geometry.coordinates[index + 1] - if (GeoOperations.distanceBetween(next, <[number, number]>projected.geometry.coordinates) < this._reusePointDistance) { + const next = outerring[index + 1] + if (GeoOperations.distanceBetween(next, projectedCoor) < this._reusePointDistance) { // We reuse this point instead! reusedPointId = this._snapOnto.nodes[index + 1] } @@ -135,8 +144,7 @@ export default class CreateNewNodeAction extends OsmCreateAction { }] } - const locations = [...this._snapOnto.coordinates] - locations.forEach(coor => coor.reverse()) + const locations = [...this._snapOnto.coordinates.map(([lat, lon]) =><[number,number]> [lon, lat])] const ids = [...this._snapOnto.nodes] locations.splice(index + 1, 0, [this._lon, this._lat]) diff --git a/Logic/Osm/Actions/CreateNewWayAction.ts b/Logic/Osm/Actions/CreateNewWayAction.ts index 68c521abc..edc6dd0bf 100644 --- a/Logic/Osm/Actions/CreateNewWayAction.ts +++ b/Logic/Osm/Actions/CreateNewWayAction.ts @@ -33,7 +33,7 @@ export default class CreateNewWayAction extends OsmCreateAction { We filter those here, as the CreateWayWithPointReuseAction delegates the actual creation to here. Filtering here also prevents similar bugs in other actions */ - if(this.coordinates.length > 0 && this.coordinates[this.coordinates.length - 1].nodeId === coordinate.nodeId){ + if(this.coordinates.length > 0 && coordinate.nodeId !== undefined && this.coordinates[this.coordinates.length - 1].nodeId === coordinate.nodeId){ // This is a duplicate id console.warn("Skipping a node in createWay to avoid a duplicate node:", coordinate,"\nThe previous coordinates are: ", this.coordinates) continue diff --git a/Logic/Osm/Actions/CreateWayWithPointReuseAction.ts b/Logic/Osm/Actions/CreateWayWithPointReuseAction.ts index afcdab4a1..95911f432 100644 --- a/Logic/Osm/Actions/CreateWayWithPointReuseAction.ts +++ b/Logic/Osm/Actions/CreateWayWithPointReuseAction.ts @@ -186,7 +186,7 @@ export default class CreateWayWithPointReuseAction extends OsmCreateAction { } public async CreateChangeDescriptions(changes: Changes): Promise { - const theme = this._state.layoutToUse.id + const theme = this._state?.layoutToUse?.id const allChanges: ChangeDescription[] = [] const nodeIdsToUse: { lat: number, lon: number, nodeId?: number }[] = [] for (let i = 0; i < this._coordinateInfo.length; i++) { @@ -251,7 +251,7 @@ export default class CreateWayWithPointReuseAction extends OsmCreateAction { const bbox = new BBox(coordinates) const state = this._state - const allNodes = [].concat(...state.featurePipeline.GetFeaturesWithin("type_node", bbox.pad(1.2))) + const allNodes = [].concat(...state?.featurePipeline?.GetFeaturesWithin("type_node", bbox.pad(1.2))??[]) const maxDistance = Math.max(...this._config.map(c => c.withinRangeOfM)) // Init coordianteinfo with undefined but the same length as coordinates diff --git a/Logic/Osm/Actions/ReplaceGeometryAction.ts b/Logic/Osm/Actions/ReplaceGeometryAction.ts index 5146e5574..f642b2d04 100644 --- a/Logic/Osm/Actions/ReplaceGeometryAction.ts +++ b/Logic/Osm/Actions/ReplaceGeometryAction.ts @@ -28,6 +28,7 @@ export default class ReplaceGeometryAction extends OsmChangeAction { /** * The target coordinates that should end up in OpenStreetMap. * This is identical to either this.feature.geometry.coordinates or -in case of a polygon- feature.geometry.coordinates[0] + * Format: [lon, lat] */ private readonly targetCoordinates: [number, number][]; /** @@ -540,8 +541,6 @@ export default class ReplaceGeometryAction extends OsmChangeAction { id: nodeId, }) }) - - } return allChanges diff --git a/Logic/Osm/Changes.ts b/Logic/Osm/Changes.ts index 10640991f..c8cfb83bc 100644 --- a/Logic/Osm/Changes.ts +++ b/Logic/Osm/Changes.ts @@ -55,8 +55,8 @@ export class Changes { // This doesn't matter however, as the '-1' is per piecewise upload, not global per changeset } - private static createChangesetFor(csId: string, - allChanges: { + static createChangesetFor(csId: string, + allChanges: { modifiedObjects: OsmObject[], newObjects: OsmObject[], deletedObjects: OsmObject[] diff --git a/Logic/Osm/OsmObject.ts b/Logic/Osm/OsmObject.ts index c73cf4e72..bb5a8e997 100644 --- a/Logic/Osm/OsmObject.ts +++ b/Logic/Osm/OsmObject.ts @@ -207,27 +207,36 @@ export abstract class OsmObject { return objects; } + /** + * Uses the list of polygon features to determine if the given tags are a polygon or not. + * */ protected static isPolygon(tags: any): boolean { for (const tagsKey in tags) { if (!tags.hasOwnProperty(tagsKey)) { continue } - const polyGuide = OsmObject.polygonFeatures.get(tagsKey) + const polyGuide : { values: Set; blacklist: boolean } = OsmObject.polygonFeatures.get(tagsKey) if (polyGuide === undefined) { continue } if ((polyGuide.values === null)) { - // We match all + // .values is null, thus merely _having_ this key is enough to be a polygon (or if blacklist, being a line) return !polyGuide.blacklist } - // is the key contained? - return polyGuide.values.has(tags[tagsKey]) + // is the key contained? Then we have a match if the value is contained + const doesMatch = polyGuide.values.has(tags[tagsKey]) + if(polyGuide.blacklist){ + return !doesMatch + } + return doesMatch } + + return false; } private static constructPolygonFeatures(): Map, blacklist: boolean }> { const result = new Map, blacklist: boolean }>(); - for (const polygonFeature of polygon_features) { + for (const polygonFeature of (polygon_features["default"] ?? polygon_features)) { const key = polygonFeature.key; if (polygonFeature.polygon === "all") { @@ -381,7 +390,7 @@ export class OsmWay extends OsmObject { } if (element.nodes === undefined) { - console.log("PANIC") + console.error("PANIC: no nodes!") } for (const nodeId of element.nodes) { @@ -417,7 +426,9 @@ export class OsmWay extends OsmObject { } private isPolygon(): boolean { - if (this.coordinates[0] !== this.coordinates[this.coordinates.length - 1]) { + // Compare lat and lon seperately, as the coordinate array might not be a reference to the same object + if (this.coordinates[0][0] !== this.coordinates[this.coordinates.length - 1][0] || + this.coordinates[0][1] !== this.coordinates[this.coordinates.length - 1][1] ) { return false; // Not closed } return OsmObject.isPolygon(this.tags) diff --git a/Logic/State/FeaturePipelineState.ts b/Logic/State/FeaturePipelineState.ts index 2c2c2a81f..62f461605 100644 --- a/Logic/State/FeaturePipelineState.ts +++ b/Logic/State/FeaturePipelineState.ts @@ -25,7 +25,7 @@ export default class FeaturePipelineState extends MapState { constructor(layoutToUse: LayoutConfig) { super(layoutToUse); - const clustering = layoutToUse.clustering + const clustering = layoutToUse?.clustering this.featureAggregator = TileHierarchyAggregator.createHierarchy(this); const clusterCounter = this.featureAggregator const self = this; diff --git a/Logic/State/MapState.ts b/Logic/State/MapState.ts index 387885a9c..32aba550d 100644 --- a/Logic/State/MapState.ts +++ b/Logic/State/MapState.ts @@ -117,10 +117,12 @@ export default class MapState extends UserRelatedState { }) - this.overlayToggles = this.layoutToUse.tileLayerSources.filter(c => c.name !== undefined).map(c => ({ + this.overlayToggles = this.layoutToUse?.tileLayerSources + ?.filter(c => c.name !== undefined) + ?.map(c => ({ config: c, isDisplayed: QueryParameters.GetBooleanQueryParameter("overlay-" + c.id, c.defaultState, "Wether or not the overlay " + c.id + " is shown") - })) + })) ?? [] this.filteredLayers = this.InitializeFilteredLayers() @@ -142,7 +144,7 @@ export default class MapState extends UserRelatedState { initialized.add(overlayToggle.config) } - for (const tileLayerSource of this.layoutToUse.tileLayerSources) { + for (const tileLayerSource of this.layoutToUse?.tileLayerSources ?? []) { if (initialized.has(tileLayerSource)) { continue } @@ -153,28 +155,14 @@ export default class MapState extends UserRelatedState { private lockBounds() { const layout = this.layoutToUse; - if (layout.lockLocation) { - if (layout.lockLocation === true) { - const tile = Tiles.embedded_tile( - layout.startLat, - layout.startLon, - layout.startZoom - 1 - ); - const bounds = Tiles.tile_bounds(tile.z, tile.x, tile.y); - // We use the bounds to get a sense of distance for this zoom level - const latDiff = bounds[0][0] - bounds[1][0]; - const lonDiff = bounds[0][1] - bounds[1][1]; - layout.lockLocation = [ - [layout.startLat - latDiff, layout.startLon - lonDiff], - [layout.startLat + latDiff, layout.startLon + lonDiff], - ]; - } - console.warn("Locking the bounds to ", layout.lockLocation); - this.mainMapObject.installBounds( - new BBox(layout.lockLocation), - this.featureSwitchIsTesting.data - ) + if (!layout?.lockLocation) { + return; } + console.warn("Locking the bounds to ", layout.lockLocation); + this.mainMapObject.installBounds( + new BBox(layout.lockLocation), + this.featureSwitchIsTesting.data + ) } private initCurrentView() { @@ -364,8 +352,10 @@ export default class MapState extends UserRelatedState { } private InitializeFilteredLayers() { - const layoutToUse = this.layoutToUse; + if(layoutToUse === undefined){ + return new UIEventSource([]) + } const flayers: FilteredLayer[] = []; for (const layer of layoutToUse.layers) { let isDisplayed: UIEventSource diff --git a/Models/ThemeConfig/PointRenderingConfig.ts b/Models/ThemeConfig/PointRenderingConfig.ts index 9bffc3d26..0a02149b8 100644 --- a/Models/ThemeConfig/PointRenderingConfig.ts +++ b/Models/ThemeConfig/PointRenderingConfig.ts @@ -127,7 +127,7 @@ export default class PointRenderingConfig extends WithContextLoader { public GetBaseIcon(tags?: any): BaseUIElement { tags = tags ?? {id: "node/-1"} const rotation = Utils.SubstituteKeys(this.rotation?.GetRenderValue(tags)?.txt ?? "0deg", tags) - const htmlDefs = Utils.SubstituteKeys(this.icon.GetRenderValue(tags)?.txt, tags) + const htmlDefs = Utils.SubstituteKeys(this.icon?.GetRenderValue(tags)?.txt, tags) let defaultPin: BaseUIElement = undefined if (this.label === undefined) { defaultPin = Svg.teardrop_with_hole_green_svg() diff --git a/Models/ThemeConfig/TagRenderingConfig.ts b/Models/ThemeConfig/TagRenderingConfig.ts index 5c1c9b28c..f0e797aae 100644 --- a/Models/ThemeConfig/TagRenderingConfig.ts +++ b/Models/ThemeConfig/TagRenderingConfig.ts @@ -338,7 +338,8 @@ export default class TagRenderingConfig { const free = this.freeform?.key if (free !== undefined) { - return tags[free] !== undefined + const value = tags[free] + return value !== undefined && value !== "" } return false diff --git a/UI/BigComponents/Histogram.ts b/UI/BigComponents/Histogram.ts index 81daeef55..d786fbf5d 100644 --- a/UI/BigComponents/Histogram.ts +++ b/UI/BigComponents/Histogram.ts @@ -93,7 +93,7 @@ export default class Histogram extends VariableUiElement { keys.sort() break; case "name-rev": - keys.sort().reverse() + keys.sort().reverse(/*Copy of array, inplace reverse if fine*/) break; case "count": keys.sort((k0, k1) => counts.get(k0) - counts.get(k1)) diff --git a/UI/Input/ValidatedTextField.ts b/UI/Input/ValidatedTextField.ts index 75b561319..9359289d4 100644 --- a/UI/Input/ValidatedTextField.ts +++ b/UI/Input/ValidatedTextField.ts @@ -543,9 +543,9 @@ class LengthTextField extends TextFieldDef { // Bit of a hack: we project the centerpoint to the closes point on the road - if available if (options?.feature !== undefined && options.feature.geometry.type !== "Point") { const lonlat = <[number, number]>[...options.location] - lonlat.reverse() + lonlat.reverse(/*Changes a clone, this is safe */) options.location = <[number, number]>GeoOperations.nearestPoint(options.feature, lonlat).geometry.coordinates - options.location.reverse() + options.location.reverse(/*Changes a clone, this is safe */) } diff --git a/UI/Popup/ImportButton.ts b/UI/Popup/ImportButton.ts index 7ad8842e8..3f956c3f0 100644 --- a/UI/Popup/ImportButton.ts +++ b/UI/Popup/ImportButton.ts @@ -373,7 +373,7 @@ export class ImportWayButton extends AbstractImportButton implements AutoAction { name: "max_snap_distance", doc: "If the imported object is a LineString or (Multi)Polygon, already existing OSM-points will be reused to construct the geometry of the newly imported way", - defaultValue: "5" + defaultValue: "0.05" }, { name: "move_osm_point_if", @@ -381,7 +381,7 @@ export class ImportWayButton extends AbstractImportButton implements AutoAction }, { name: "max_move_distance", doc: "If an OSM-point is moved, the maximum amount of meters it is moved. Capped on 20m", - defaultValue: "1" + defaultValue: "0.05" }, { name: "snap_onto_layers", doc: "If no existing nearby point exists, but a line of a specified layer is closeby, snap to this layer instead", @@ -406,24 +406,12 @@ export class ImportWayButton extends AbstractImportButton implements AutoAction AbstractImportButton.importedIds.add(originalFeatureTags.data.id) const args = this.parseArgs(argument, originalFeatureTags) const feature = state.allElements.ContainingFeatures.get(id) - console.log("Geometry to auto-import is:", feature) - const geom = feature.geometry - let coordinates: [number, number][] - if (geom.type === "LineString") { - coordinates = geom.coordinates - } else if (geom.type === "Polygon") { - coordinates = geom.coordinates[0] - } - - const mergeConfigs = this.GetMergeConfig(args); - - const action = this.CreateAction( + const action = ImportWayButton.CreateAction( feature, args, state, - mergeConfigs, - coordinates + mergeConfigs ) await state.changes.applyAction(action) } @@ -455,18 +443,8 @@ export class ImportWayButton extends AbstractImportButton implements AutoAction // Upload the way to OSM - const geom = feature.geometry - let coordinates: [number, number][] - if (geom.type === "LineString") { - coordinates = geom.coordinates - } else if (geom.type === "Polygon") { - coordinates = geom.coordinates[0] - } const mergeConfigs = this.GetMergeConfig(args); - - - let action = this.CreateAction(feature, args, state, mergeConfigs, coordinates); - + let action = ImportWayButton.CreateAction(feature, args, state, mergeConfigs); return this.createConfirmPanelForWay( state, args, @@ -508,14 +486,12 @@ export class ImportWayButton extends AbstractImportButton implements AutoAction return mergeConfigs; } - private CreateAction(feature, + private static CreateAction(feature, args: { max_snap_distance: string; snap_onto_layers: string; icon: string; text: string; tags: string; newTags: UIEventSource; targetLayer: string }, state: FeaturePipelineState, - mergeConfigs: any[], - coordinates: [number, number][]) { - + mergeConfigs: any[]) { const coors = feature.geometry.coordinates - if (feature.geometry.type === "Polygon" && coors.length > 1) { + if ((feature.geometry.type === "Polygon" ) && coors.length > 1) { const outer = coors[0] const inner = [...coors] inner.splice(0, 1) @@ -531,7 +507,7 @@ export class ImportWayButton extends AbstractImportButton implements AutoAction return new CreateWayWithPointReuseAction( args.newTags.data, - coordinates, + coors, state, mergeConfigs ) diff --git a/UI/Popup/LoginButton.ts b/UI/Popup/LoginButton.ts index dd16d6411..ecf859daf 100644 --- a/UI/Popup/LoginButton.ts +++ b/UI/Popup/LoginButton.ts @@ -28,7 +28,6 @@ export class LoginToggle extends VariableUiElement { const login = new LoginButton(text, state) super( state.osmConnection.loadingStatus.map(osmConnectionState => { - console.trace("Current osm state is ", osmConnectionState) if(osmConnectionState === "loading"){ return loading } diff --git a/UI/Popup/SplitRoadWizard.ts b/UI/Popup/SplitRoadWizard.ts index 5bdd70667..8ae8b0158 100644 --- a/UI/Popup/SplitRoadWizard.ts +++ b/UI/Popup/SplitRoadWizard.ts @@ -107,7 +107,7 @@ export default class SplitRoadWizard extends Toggle { .filter(p => GeoOperations.distanceBetween(p[0].geometry.coordinates, coordinates) < 5) .map(p => p[1]) .sort((a, b) => a - b) - .reverse() + .reverse(/*Copy/derived list, inplace reverse is fine*/) if (points.length > 0) { for (const point of points) { splitPoints.data.splice(point, 1) diff --git a/Utils.ts b/Utils.ts index 41e4d141e..a39f34e88 100644 --- a/Utils.ts +++ b/Utils.ts @@ -777,5 +777,6 @@ In the case that MapComplete is pointed to the testing grounds, the edit will be b: parseInt(hex.substr(5, 2), 16), } } + } diff --git a/test/CodeQuality.spec.ts b/test/CodeQuality.spec.ts index 9962892e1..7b3ce78c5 100644 --- a/test/CodeQuality.spec.ts +++ b/test/CodeQuality.spec.ts @@ -2,34 +2,48 @@ import T from "./TestHelper"; import {exec} from "child_process"; export default class CodeQualitySpec extends T { + constructor() { super([ [ "no constructor.name in compiled code", () => { - - const excludedDirs = [".git", "node_modules", "dist", ".cache", ".parcel-cache", "assets"] - - exec("grep \"constructor.name\" -r . " + excludedDirs.map(d => "--exclude-dir=" + d).join(" "), ((error, stdout, stderr) => { - if (error?.message?.startsWith("Command failed: grep")) { - return; - } - if (error !== null) { - throw error - - } - if (stderr !== "") { - throw stderr - } - - const found = stdout.split("\n").filter(s => s !== "").filter(s => s.startsWith("test/")); - if (found.length > 0) { - throw "Found a 'constructor.name' at " + found.join(", ") + ". This is not allowed, as minification does erase names." - } - - })) - - } - ] + CodeQualitySpec.detectInCode("constructor\\.name", "This is not allowed, as minification does erase names.") + }], + [ + "no reverse in compiled code", () => { + CodeQualitySpec.detectInCode("reverse()", "Reverse is stateful and changes the source list. This often causes subtle bugs") + }] ]); } + + /** + * + * @param forbidden: a GREP-regex. This means that '.' is a wildcard and should be escaped to match a literal dot + * @param reason + * @private + */ + private static detectInCode(forbidden: string, reason: string) { + + const excludedDirs = [".git", "node_modules", "dist", ".cache", ".parcel-cache", "assets"] + + exec("grep -n \"" + forbidden + "\" -r . " + excludedDirs.map(d => "--exclude-dir=" + d).join(" "), ((error, stdout, stderr) => { + if (error?.message?.startsWith("Command failed: grep")) { + console.warn("Command failed!") + return; + } + if (error !== null) { + throw error + + } + if (stderr !== "") { + throw stderr + } + + const found = stdout.split("\n").filter(s => s !== "").filter(s => !s.startsWith("./test/")); + if (found.length > 0) { + throw `Found a '${forbidden}' at \n ${found.join("\n ")}.\n ${reason}` + } + + })) + } } \ No newline at end of file diff --git a/test/ImportMultiPolygon.spec.ts b/test/ImportMultiPolygon.spec.ts new file mode 100644 index 000000000..e4b480f92 --- /dev/null +++ b/test/ImportMultiPolygon.spec.ts @@ -0,0 +1,219 @@ +import T from "./TestHelper"; +import CreateMultiPolygonWithPointReuseAction from "../Logic/Osm/Actions/CreateMultiPolygonWithPointReuseAction"; +import { Tag } from "../Logic/Tags/Tag"; +import FeaturePipelineState from "../Logic/State/FeaturePipelineState"; +import { Changes } from "../Logic/Osm/Changes"; +import {ChangesetHandler} from "../Logic/Osm/ChangesetHandler"; +import * as Assert from "assert"; + +export default class ImportMultiPolygonSpec extends T { + + constructor() { + super([ + ["Correct changeset", + async () => { + + const feature = { + "type": "Feature", + "properties": { + "osm_id": "41097039", + "size_grb_building": "1374.89", + "addr:housenumber": "53", + "addr:street": "Startelstraat", + "building": "house", + "source:geometry:entity": "Gbg", + "source:geometry:date": "2014-04-28", + "source:geometry:oidn": "150044", + "source:geometry:uidn": "5403181", + "H_DTM_MIN": "50.35", + "H_DTM_GEM": "50.97", + "H_DSM_MAX": "59.40", + "H_DSM_P99": "59.09", + "HN_MAX": "8.43", + "HN_P99": "8.12", + "detection_method": "derived from OSM landuse: farmyard", + "auto_target_landuse": "farmyard", + "size_source_landuse": "8246.28", + "auto_building": "farm", + "id": "41097039", + "_lat": "50.84633355000016", + "_lon": "5.262964150000011", + "_layer": "grb", + "_length": "185.06002152312757", + "_length:km": "0.2", + "_now:date": "2022-02-22", + "_now:datetime": "2022-02-22 10:15:51", + "_loaded:date": "2022-02-22", + "_loaded:datetime": "2022-02-22 10:15:51", + "_geometry:type": "Polygon", + "_intersects_with_other_features": "", + "_country": "be", + "_overlaps_with_buildings": "[]", + "_overlap_percentage": "null", + "_grb_date": "2014-04-28", + "_grb_ref": "Gbg/150044", + "_building:min_level": "", + "_surface": "548.1242491529038", + "_surface:ha": "0", + "_reverse_overlap_percentage": "null", + "_imported_osm_object_found": "false", + "_imported_osm_still_fresh": "false", + "_target_building_type": "house" + }, + "geometry": { + "type": "Polygon", + "coordinates": <[number, number][][]>[ + [ + [ + 5.262684300000043, + 50.84624409999995 + ], + [ + 5.262777500000024, + 50.84620759999988 + ], + [ + 5.262798899999998, + 50.84621390000019 + ], + [ + 5.262999799999994, + 50.84619519999999 + ], + [ + 5.263107500000007, + 50.84618920000014 + ], + [ + 5.263115, + 50.84620990000026 + ], + [ + 5.26310279999998, + 50.84623050000014 + ], + [ + 5.263117999999977, + 50.846247400000166 + ], + [ + 5.263174599999989, + 50.84631019999971 + ], + [ + 5.263166999999989, + 50.84631459999995 + ], + [ + 5.263243999999979, + 50.84640239999989 + ], + [ + 5.2631607000000065, + 50.84643459999996 + ], + [ + 5.26313309999997, + 50.84640089999985 + ], + [ + 5.262907499999996, + 50.84647790000018 + ], + [ + 5.2628939999999576, + 50.846463699999774 + ], + [ + 5.262872100000033, + 50.846440700000294 + ], + [ + 5.262784699999991, + 50.846348899999924 + ], + [ + 5.262684300000043, + 50.84624409999995 + ] + ], + [ + [ + 5.262801899999976, + 50.84623269999982 + ], + [ + 5.2629535000000285, + 50.84638830000012 + ], + [ + 5.263070700000018, + 50.84634720000008 + ], + [ + 5.262998000000025, + 50.84626279999982 + ], + [ + 5.263066799999966, + 50.84623959999975 + ], + [ + 5.263064000000004, + 50.84623330000007 + ], + [ + 5.263009599999997, + 50.84623730000026 + ], + [ + 5.263010199999956, + 50.84621629999986 + ], + [ + 5.262801899999976, + 50.84623269999982 + ] + ] + ] + }, + } + + const innerRings = [...feature.geometry.coordinates] + innerRings.splice(0, 1) + + const action = new CreateMultiPolygonWithPointReuseAction( + [new Tag("building", "yes")], + feature.geometry.coordinates[0], + innerRings, + undefined, + [], + "import" + ) + const descriptions = await action.Perform(new Changes()) + + function getCoor(id: number): {lat: number, lon:number} { + return descriptions.find(d => d.type === "node" && d.id === id).changes + } + + const ways= descriptions.filter(d => d.type === "way") + T.isTrue(ways[0].id == -18, "unexpected id") + T.isTrue(ways[1].id == -27, "unexpected id") + const outer = ways[0].changes["coordinates"] + const outerExpected = [[5.262684300000043,50.84624409999995],[5.262777500000024,50.84620759999988],[5.262798899999998,50.84621390000019],[5.262999799999994,50.84619519999999],[5.263107500000007,50.84618920000014],[5.263115,50.84620990000026],[5.26310279999998,50.84623050000014],[5.263117999999977,50.846247400000166],[5.263174599999989,50.84631019999971],[5.263166999999989,50.84631459999995],[5.263243999999979,50.84640239999989],[5.2631607000000065,50.84643459999996],[5.26313309999997,50.84640089999985],[5.262907499999996,50.84647790000018],[5.2628939999999576,50.846463699999774],[5.262872100000033,50.846440700000294],[5.262784699999991,50.846348899999924],[5.262684300000043,50.84624409999995]] + T.listIdentical(feature.geometry.coordinates[0], outer) + const inner = ways[1].changes["coordinates"] + T.listIdentical(feature.geometry.coordinates[1], inner) + const members = <{type: string, role: string, ref: number}[]> descriptions.find(d => d.type === "relation").changes["members"] + T.isTrue(members[0].role == "outer", "incorrect role") + T.isTrue(members[1].role == "inner", "incorrect role") + T.isTrue(members[0].type == "way", "incorrect type") + T.isTrue(members[1].type == "way", "incorrect type") + T.isTrue(members[0].ref == -18, "incorrect id") + T.isTrue(members[1].ref == -27, "incorrect id") + }] + ]); + } + + +} \ No newline at end of file diff --git a/test/TestAll.ts b/test/TestAll.ts index 5d43853cc..37f408220 100644 --- a/test/TestAll.ts +++ b/test/TestAll.ts @@ -20,6 +20,7 @@ import CreateNoteImportLayerSpec from "./CreateNoteImportLayer.spec"; import ValidatedTextFieldTranslationsSpec from "./ValidatedTextFieldTranslations.spec"; import CreateCacheSpec from "./CreateCache.spec"; import CodeQualitySpec from "./CodeQuality.spec"; +import ImportMultiPolygonSpec from "./ImportMultiPolygon.spec"; async function main() { @@ -43,7 +44,8 @@ async function main() { new CreateNoteImportLayerSpec(), new ValidatedTextFieldTranslationsSpec(), new CreateCacheSpec(), - new CodeQualitySpec() + new CodeQualitySpec(), + new ImportMultiPolygonSpec() ] ScriptUtils.fixUtils(); const realDownloadFunc = Utils.externalDownloadFunction;