Skip to content

Commit

Permalink
Merge pull request #6232 from entur/otp2_npe_situation_url
Browse files Browse the repository at this point in the history
Return empty list if there is no siriUrls in situations/infoLinks
  • Loading branch information
t2gran authored Nov 7, 2024
2 parents 87b6a46 + f492446 commit 2ead4da
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public static GraphQLObjectType create() {
.description("URI")
.dataFetcher(environment -> {
AlertUrl source = environment.getSource();
return source.uri;
return source.uri();
})
.build()
)
Expand All @@ -32,7 +32,7 @@ public static GraphQLObjectType create() {
.description("Label")
.dataFetcher(environment -> {
AlertUrl source = environment.getSource();
return source.label;
return source.label();
})
.build()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public static GraphQLObjectType create(
if (!siriUrls.isEmpty()) {
return siriUrls;
}
return null;
return emptyList();
})
.build()
)
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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();
}

/**
Expand All @@ -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 + "'")
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,9 @@ 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: {}",
Expand Down Expand Up @@ -221,18 +221,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));
}

/*
* 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);
Expand All @@ -247,21 +247,32 @@ private I18NString getInfoLinkAsString(PtSituationElement.InfoLinks infoLinks) {
/*
* Returns all InfoLinks
*/
private List<AlertUrl> getInfoLinks(PtSituationElement.InfoLinks infoLinks) {
private List<AlertUrl> mapInfoLinks(PtSituationElement situation) {
PtSituationElement.InfoLinks infoLinks = situation.getInfoLinks();
List<AlertUrl> alertUrls = new ArrayList<>();
if (infoLinks != null) {
if (isNotEmpty(infoLinks.getInfoLinks())) {
for (InfoLinkStructure infoLink : infoLinks.getInfoLinks()) {
AlertUrl alertUrl = new AlertUrl();

String label = null;
List<NaturalLanguageStringStructure> 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));
} else {
if (LOG.isDebugEnabled()) {
LOG.debug(
"URI missing in info-link - ignoring info-link in situation: {}",
situation.getSituationNumber() != null
? situation.getSituationNumber().getValue()
: null
);
}
}
}
}
}
Expand All @@ -281,7 +292,7 @@ private boolean isNotEmpty(List<?> list) {
*
* @return A TranslatedString containing the same information as the input
*/
private I18NString getTranslatedString(List<DefaultedTextStructure> input) {
private I18NString mapTranslatedString(List<DefaultedTextStructure> input) {
Map<String, String> translations = new HashMap<>();
if (input != null && input.size() > 0) {
for (DefaultedTextStructure textStructure : input) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,8 @@ public void testSiriSxUpdateForStop() {

final List<AlertUrl> 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
Expand Down

0 comments on commit 2ead4da

Please sign in to comment.