Merge pull request #6049 from HSLdevcom/fix-floornumber-report

OSM data links added to the graph build report about ambiguous levels and layers
opentripplanner · Sep 10, 2024 · 3b9f3a2 · 3b9f3a2
2 parents 6e9daaf + 8cedc68
commit 3b9f3a2
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 32 deletions.
diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java
@@ -651,10 +651,12 @@ private void applyLevelsForWay(OSMWithTags way) {
       OSMLevel level = OSMLevel.DEFAULT;
       if (way.hasTag("level")) { // TODO: floating-point levels &c.
         levelName = way.getTag("level");
-        level = OSMLevel.fromString(levelName, OSMLevel.Source.LEVEL_TAG, noZeroLevels, issueStore);
+        level =
+          OSMLevel.fromString(levelName, OSMLevel.Source.LEVEL_TAG, noZeroLevels, issueStore, way);
       } else if (way.hasTag("layer")) {
         levelName = way.getTag("layer");
-        level = OSMLevel.fromString(levelName, OSMLevel.Source.LAYER_TAG, noZeroLevels, issueStore);
+        level =
+          OSMLevel.fromString(levelName, OSMLevel.Source.LAYER_TAG, noZeroLevels, issueStore, way);
       }
       if (level == null || (!level.reliable)) {
         issueStore.add(new LevelAmbiguous(levelName, way));
@@ -980,7 +982,8 @@ private void processLevelMap(OSMRelation relation) {
       levelsTag,
       Source.LEVEL_MAP,
       true,
-      issueStore
+      issueStore,
+      relation
     );
     for (OSMRelationMember member : relation.getMembers()) {
       if (member.hasTypeWay() && waysById.containsKey(member.getRef())) {

diff --git a/...n/java/org/opentripplanner/openstreetmap/issues/FloorNumberUnknownAssumedGroundLevel.java b/...n/java/org/opentripplanner/openstreetmap/issues/FloorNumberUnknownAssumedGroundLevel.java
@@ -1,22 +1,23 @@
 package org.opentripplanner.openstreetmap.issues;
 
 import org.opentripplanner.graph_builder.issue.api.DataImportIssue;
+import org.opentripplanner.openstreetmap.model.OSMWithTags;
 
-public class FloorNumberUnknownAssumedGroundLevel implements DataImportIssue {
+public record FloorNumberUnknownAssumedGroundLevel(String layer, OSMWithTags entity)
+  implements DataImportIssue {
+  private static final String FMT =
+    "%s : could not determine floor number for layer %s, assumed to be ground-level.";
 
-  public static final String FMT =
-    "Could not determine floor number for layer %s, assumed to be ground-level.";
+  private static final String HTMLFMT =
+    "<a href='%s'>'%s'</a> : could not determine floor number for layer %s, assumed to be ground-level.";
 
-  final String layer;
-  final Integer floorNumber;
-
-  public FloorNumberUnknownAssumedGroundLevel(String layer, Integer floorNumber) {
-    this.layer = layer;
-    this.floorNumber = floorNumber;
+  @Override
+  public String getMessage() {
+    return String.format(FMT, entity.getId(), layer);
   }
 
   @Override
-  public String getMessage() {
-    return String.format(FMT, layer, floorNumber);
+  public String getHTMLMessage() {
+    return String.format(HTMLFMT, entity.url(), entity.getId(), layer);
   }
 }
diff --git a/.../java/org/opentripplanner/openstreetmap/issues/FloorNumberUnknownGuessedFromAltitude.java b/.../java/org/opentripplanner/openstreetmap/issues/FloorNumberUnknownGuessedFromAltitude.java
@@ -1,22 +1,27 @@
 package org.opentripplanner.openstreetmap.issues;
 
 import org.opentripplanner.graph_builder.issue.api.DataImportIssue;
+import org.opentripplanner.openstreetmap.model.OSMWithTags;
 
-public class FloorNumberUnknownGuessedFromAltitude implements DataImportIssue {
+public record FloorNumberUnknownGuessedFromAltitude(
+  String layer,
+  Integer floorNumber,
+  OSMWithTags entity
+)
+  implements DataImportIssue {
+  private static final String FMT =
+    "%s : could not determine floor number for layer %s. Guessed %s (0-based) from altitude.";
 
-  public static final String FMT =
-    "Could not determine floor number for layer %s. Guessed %s (0-based) from altitude.";
+  private static final String HTMLFMT =
+    "<a href='%s'>'%s'</a> : could not determine floor number for layer %s. Guessed %s (0-based) from altitude.";
 
-  final String layer;
-  final Integer floorNumber;
-
-  public FloorNumberUnknownGuessedFromAltitude(String layer, Integer floorNumber) {
-    this.layer = layer;
-    this.floorNumber = floorNumber;
+  @Override
+  public String getMessage() {
+    return String.format(FMT, entity.getId(), layer, floorNumber);
   }
 
   @Override
-  public String getMessage() {
-    return String.format(FMT, layer, floorNumber);
+  public String getHTMLMessage() {
+    return String.format(HTMLFMT, entity.url(), entity.getId(), layer, floorNumber);
   }
 }
diff --git a/src/main/java/org/opentripplanner/openstreetmap/model/OSMLevel.java b/src/main/java/org/opentripplanner/openstreetmap/model/OSMLevel.java
@@ -53,7 +53,8 @@ public static OSMLevel fromString(
     String spec,
     Source source,
     boolean incrementNonNegative,
-    DataImportIssueStore issueStore
+    DataImportIssueStore issueStore,
+    OSMWithTags osmObj
   ) {
     /*  extract any altitude information after the @ character */
     Double altitude = null;
@@ -111,7 +112,7 @@ public static OSMLevel fromString(
     /* fall back on altitude when necessary */
     if (floorNumber == null && altitude != null) {
       floorNumber = (int) (altitude / METERS_PER_FLOOR);
-      issueStore.add(new FloorNumberUnknownGuessedFromAltitude(spec, floorNumber));
+      issueStore.add(new FloorNumberUnknownGuessedFromAltitude(spec, floorNumber, osmObj));
       reliable = false;
     }
 
@@ -122,7 +123,7 @@ public static OSMLevel fromString(
     /* signal failure to extract any useful level information */
     if (floorNumber == null) {
       floorNumber = 0;
-      issueStore.add(new FloorNumberUnknownAssumedGroundLevel(spec, floorNumber));
+      issueStore.add(new FloorNumberUnknownAssumedGroundLevel(spec, osmObj));
       reliable = false;
     }
     return new OSMLevel(floorNumber, altitude, shortName, longName, source, reliable);
@@ -132,7 +133,8 @@ public static List<OSMLevel> fromSpecList(
     String specList,
     Source source,
     boolean incrementNonNegative,
-    DataImportIssueStore issueStore
+    DataImportIssueStore issueStore,
+    OSMWithTags osmObj
   ) {
     List<String> levelSpecs = new ArrayList<>();
 
@@ -153,7 +155,7 @@ public static List<OSMLevel> fromSpecList(
     /* build an OSMLevel for each level spec in the list */
     List<OSMLevel> levels = new ArrayList<>();
     for (String spec : levelSpecs) {
-      levels.add(fromString(spec, source, incrementNonNegative, issueStore));
+      levels.add(fromString(spec, source, incrementNonNegative, issueStore, osmObj));
     }
     return levels;
   }
@@ -162,10 +164,17 @@ public static Map<String, OSMLevel> mapFromSpecList(
     String specList,
     Source source,
     boolean incrementNonNegative,
-    DataImportIssueStore issueStore
+    DataImportIssueStore issueStore,
+    OSMWithTags osmObj
   ) {
     Map<String, OSMLevel> map = new HashMap<>();
-    for (OSMLevel level : fromSpecList(specList, source, incrementNonNegative, issueStore)) {
+    for (OSMLevel level : fromSpecList(
+      specList,
+      source,
+      incrementNonNegative,
+      issueStore,
+      osmObj
+    )) {
       map.put(level.shortName, level);
     }
     return map;