From 021893f7cb8811932f94200b67b2b18cc6f132ed Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Wed, 6 Nov 2024 11:49:19 +0100 Subject: [PATCH] Ignore situation alerts without url or siri uris --- .../model/framework/InfoLinkType.java | 4 +- .../framework/i18n/I18NString.java | 22 +++++++-- .../routing/alertpatch/AlertUrl.java | 9 ++-- .../updater/siri/SiriAlertsUpdateHandler.java | 47 +++++++++++-------- .../framework/i18n/I18NStringTest.java | 32 +++++++++++++ .../siri/SiriAlertsUpdateHandlerTest.java | 4 +- 6 files changed, 87 insertions(+), 31 deletions(-) create mode 100644 application/src/test/java/org/opentripplanner/framework/i18n/I18NStringTest.java diff --git a/application/src/main/java/org/opentripplanner/apis/transmodel/model/framework/InfoLinkType.java b/application/src/main/java/org/opentripplanner/apis/transmodel/model/framework/InfoLinkType.java index 6541b48a44d..e0472a2ccf8 100644 --- a/application/src/main/java/org/opentripplanner/apis/transmodel/model/framework/InfoLinkType.java +++ b/application/src/main/java/org/opentripplanner/apis/transmodel/model/framework/InfoLinkType.java @@ -20,7 +20,7 @@ public static GraphQLObjectType create() { .description("URI") .dataFetcher(environment -> { AlertUrl source = environment.getSource(); - return source.uri; + return source.uri(); }) .build() ) @@ -32,7 +32,7 @@ public static GraphQLObjectType create() { .description("Label") .dataFetcher(environment -> { AlertUrl source = environment.getSource(); - return source.label; + return source.label(); }) .build() ) diff --git a/application/src/main/java/org/opentripplanner/framework/i18n/I18NString.java b/application/src/main/java/org/opentripplanner/framework/i18n/I18NString.java index 4f75d214c91..101342c7a6f 100644 --- a/application/src/main/java/org/opentripplanner/framework/i18n/I18NString.java +++ b/application/src/main/java/org/opentripplanner/framework/i18n/I18NString.java @@ -1,6 +1,7 @@ package org.opentripplanner.framework.i18n; import java.util.Locale; +import javax.annotation.Nullable; /** * This interface is used when providing translations on server side. Sources: OSM tags with @@ -9,9 +10,20 @@ * @author mabu */ public interface I18NString { - /** true if the given value is not {@code null} or has at least one none white-space character. */ - public static boolean hasValue(I18NString value) { - return value != null && !value.toString().isBlank(); + /** + * Return {@code true} if the given value is not {@code null} or has at least one none + * white-space character. + */ + static boolean hasValue(@Nullable I18NString value) { + return !hasNoValue(value); + } + + /** + * Return {@code true} if the given value has at least one none white-space character. + * Return {@code false} if the value is {@code null} or blank. + */ + static boolean hasNoValue(@Nullable I18NString value) { + return value == null || value.toString().isBlank(); } /** @@ -26,8 +38,8 @@ public static boolean hasValue(I18NString value) { */ String toString(Locale locale); - static I18NString assertHasValue(I18NString value) { - if (value == null || value.toString().isBlank()) { + static I18NString assertHasValue(@Nullable I18NString value) { + if (hasNoValue(value)) { throw new IllegalArgumentException( "Value can not be null, empty or just whitespace: " + (value == null ? "null" : "'" + value + "'") diff --git a/application/src/main/java/org/opentripplanner/routing/alertpatch/AlertUrl.java b/application/src/main/java/org/opentripplanner/routing/alertpatch/AlertUrl.java index b13fad6e97b..fcce3720538 100644 --- a/application/src/main/java/org/opentripplanner/routing/alertpatch/AlertUrl.java +++ b/application/src/main/java/org/opentripplanner/routing/alertpatch/AlertUrl.java @@ -1,7 +1,10 @@ package org.opentripplanner.routing.alertpatch; -public class AlertUrl { +import java.util.Objects; +import javax.annotation.Nullable; - public String uri; - public String label; +public record AlertUrl(String uri, @Nullable String label) { + public AlertUrl { + Objects.requireNonNull(uri); + } } diff --git a/application/src/main/java/org/opentripplanner/updater/siri/SiriAlertsUpdateHandler.java b/application/src/main/java/org/opentripplanner/updater/siri/SiriAlertsUpdateHandler.java index a5817eca41b..9d39af602e0 100644 --- a/application/src/main/java/org/opentripplanner/updater/siri/SiriAlertsUpdateHandler.java +++ b/application/src/main/java/org/opentripplanner/updater/siri/SiriAlertsUpdateHandler.java @@ -135,12 +135,20 @@ private TransitAlert mapSituationToAlert( TransitAlertBuilder alert = createAlertWithTexts(situation); if ( - (alert.headerText() == null || alert.headerText().toString().isEmpty()) && - (alert.descriptionText() == null || alert.descriptionText().toString().isEmpty()) && - (alert.detailText() == null || alert.detailText().toString().isEmpty()) + I18NString.hasNoValue(alert.headerText()) && + I18NString.hasNoValue(alert.descriptionText()) && + I18NString.hasNoValue(alert.detailText()) ) { LOG.debug( - "Empty Alert - ignoring situationNumber: {}", + "Empty Alert, no text - ignoring situationNumber: {}", + situation.getSituationNumber() != null ? situation.getSituationNumber().getValue() : null + ); + return null; + } + + if (I18NString.hasNoValue(alert.url()) && alert.siriUrls().isEmpty()) { + LOG.debug( + "Empty Alert, no uris - ignoring situationNumber: {}", situation.getSituationNumber() != null ? situation.getSituationNumber().getValue() : null ); return null; @@ -221,18 +229,18 @@ private long getEpochSecond(ZonedDateTime startTime) { private TransitAlertBuilder createAlertWithTexts(PtSituationElement situation) { return TransitAlert .of(new FeedScopedId(feedId, situation.getSituationNumber().getValue())) - .withDescriptionText(getTranslatedString(situation.getDescriptions())) - .withDetailText(getTranslatedString(situation.getDetails())) - .withAdviceText(getTranslatedString(situation.getAdvices())) - .withHeaderText(getTranslatedString(situation.getSummaries())) - .withUrl(getInfoLinkAsString(situation.getInfoLinks())) - .addSiriUrls(getInfoLinks(situation.getInfoLinks())); + .withDescriptionText(mapTranslatedString(situation.getDescriptions())) + .withDetailText(mapTranslatedString(situation.getDetails())) + .withAdviceText(mapTranslatedString(situation.getAdvices())) + .withHeaderText(mapTranslatedString(situation.getSummaries())) + .withUrl(mapInfoLinkToI18NString(situation.getInfoLinks())) + .addSiriUrls(mapInfoLinks(situation.getInfoLinks())); } /* * Returns first InfoLink-uri as a String */ - private I18NString getInfoLinkAsString(PtSituationElement.InfoLinks infoLinks) { + private I18NString mapInfoLinkToI18NString(PtSituationElement.InfoLinks infoLinks) { if (infoLinks != null) { if (isNotEmpty(infoLinks.getInfoLinks())) { InfoLinkStructure infoLinkStructure = infoLinks.getInfoLinks().get(0); @@ -247,21 +255,22 @@ private I18NString getInfoLinkAsString(PtSituationElement.InfoLinks infoLinks) { /* * Returns all InfoLinks */ - private List getInfoLinks(PtSituationElement.InfoLinks infoLinks) { + private List mapInfoLinks(PtSituationElement.InfoLinks infoLinks) { List alertUrls = new ArrayList<>(); if (infoLinks != null) { if (isNotEmpty(infoLinks.getInfoLinks())) { for (InfoLinkStructure infoLink : infoLinks.getInfoLinks()) { - AlertUrl alertUrl = new AlertUrl(); - + String label = null; List labels = infoLink.getLabels(); if (labels != null && !labels.isEmpty()) { - NaturalLanguageStringStructure label = labels.get(0); - alertUrl.label = label.getValue(); + NaturalLanguageStringStructure lbl = labels.get(0); + label = lbl.getValue(); } - alertUrl.uri = infoLink.getUri(); - alertUrls.add(alertUrl); + var uri = infoLink.getUri(); + if (uri != null) { + alertUrls.add(new AlertUrl(uri, label)); + } } } } @@ -281,7 +290,7 @@ private boolean isNotEmpty(List list) { * * @return A TranslatedString containing the same information as the input */ - private I18NString getTranslatedString(List input) { + private I18NString mapTranslatedString(List input) { Map translations = new HashMap<>(); if (input != null && input.size() > 0) { for (DefaultedTextStructure textStructure : input) { diff --git a/application/src/test/java/org/opentripplanner/framework/i18n/I18NStringTest.java b/application/src/test/java/org/opentripplanner/framework/i18n/I18NStringTest.java new file mode 100644 index 00000000000..101da6edc91 --- /dev/null +++ b/application/src/test/java/org/opentripplanner/framework/i18n/I18NStringTest.java @@ -0,0 +1,32 @@ +package org.opentripplanner.framework.i18n; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class I18NStringTest { + + private final I18NString noValue = I18NString.of(" \t\n\r\f"); + private final I18NString hasValue = I18NString.of("A value"); + + @Test + void hasValue() { + assertTrue(I18NString.hasValue(hasValue)); + assertFalse(I18NString.hasValue(noValue)); + } + + @Test + void hasNoValue() { + assertFalse(I18NString.hasNoValue(hasValue)); + assertTrue(I18NString.hasNoValue(noValue)); + } + + @Test + void assertHasValue() { + var ex = assertThrows(IllegalArgumentException.class, () -> I18NString.assertHasValue(noValue)); + assertEquals("Value can not be null, empty or just whitespace: ' \t\n\r\f'", ex.getMessage()); + } +} diff --git a/application/src/test/java/org/opentripplanner/updater/siri/SiriAlertsUpdateHandlerTest.java b/application/src/test/java/org/opentripplanner/updater/siri/SiriAlertsUpdateHandlerTest.java index 94c4af8d22e..3cd8e50c000 100644 --- a/application/src/test/java/org/opentripplanner/updater/siri/SiriAlertsUpdateHandlerTest.java +++ b/application/src/test/java/org/opentripplanner/updater/siri/SiriAlertsUpdateHandlerTest.java @@ -198,8 +198,8 @@ public void testSiriSxUpdateForStop() { final List alertUrlList = transitAlert.siriUrls(); AlertUrl alertUrl = alertUrlList.get(0); - assertEquals(infoLinkUri, alertUrl.uri); - assertEquals(infoLinkLabel, alertUrl.label); + assertEquals(infoLinkUri, alertUrl.uri()); + assertEquals(infoLinkLabel, alertUrl.label()); } @Test