Apply review feedback

Author: leonardehrenfried

application/src/main/java/org/opentripplanner/gtfs/mapping/NoticeAssignmentMapper.java

@@ -49,16 +49,16 @@ Multimap<AbstractTransitEntity, Notice> map(Collection<NoticeAssignment> assignm
       .collect(Collectors.toMap(Trip::getId, Function.identity()));
     var routes = routeMapper.mappedRoutes();
     for (var assignment : assignments) {
-      mapOne(noticeMapper.mappedNotices(), assignment, trips, routes).ifPresent(entry ->
+      mapOne(assignment, noticeMapper.mappedNotices(), trips, routes).ifPresent(entry ->
         result.put(entry.getKey(), entry.getValue())
       );
     }
     return result;
   }
 
   private Optional<Map.Entry<AbstractTransitEntity, Notice>> mapOne(
-    Map<FeedScopedId, Notice> notices,
     NoticeAssignment assignment,
+    Map<FeedScopedId, Notice> notices,
     Map<FeedScopedId, Trip> trips,
     Map<FeedScopedId, Route> routes
   ) {

application/src/test/java/org/opentripplanner/gtfs/mapping/NoticeAssignmentMapperTest.java

@@ -1,7 +1,7 @@
 package org.opentripplanner.gtfs.mapping;
 
+import static com.google.common.truth.Truth.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.onebusaway.gtfs.model.AgencyAndIdFactory.obaId;
 
 import java.util.List;
@@ -111,13 +111,14 @@ void mapNoticeAssignmentOnTrip() {
   void missingNoticeLogsIssue() {
     var issues = new DefaultDataImportIssueStore();
     var noticeMapper = new NoticeMapper(ID_FACTORY);
+    var routeMapper = createRouteMapper();
 
     var mapper = new NoticeAssignmentMapper(
       ID_FACTORY,
       issues,
       noticeMapper,
-      createTripMapper(createRouteMapper()),
-      createRouteMapper()
+      createTripMapper(routeMapper),
+      routeMapper
     );
 
     var assignment = new NoticeAssignment();
@@ -127,10 +128,9 @@ void missingNoticeLogsIssue() {
 
     var result = mapper.map(List.of(assignment));
 
-    assertTrue(result.isEmpty());
-    assertEquals(
-      List.of("NoticeAssignmentWithoutNotice"),
-      issues.listIssues().stream().map(DataImportIssue::getType).toList()
+    assertThat(result).isEmpty();
+    assertThat(issues.listIssues().stream().map(DataImportIssue::getType)).containsExactly(
+      "NoticeAssignmentWithoutNotice"
     );
   }
 
@@ -139,13 +139,14 @@ void missingRouteEntityLogsIssue() {
     var issues = new DefaultDataImportIssueStore();
     var noticeMapper = new NoticeMapper(ID_FACTORY);
     noticeMapper.map(GTFS_NOTICE);
+    var routeMapper = createRouteMapper();
 
     var mapper = new NoticeAssignmentMapper(
       ID_FACTORY,
       issues,
       noticeMapper,
-      createTripMapper(createRouteMapper()),
-      createRouteMapper()
+      createTripMapper(routeMapper),
+      routeMapper
     );
 
     var assignment = new NoticeAssignment();
@@ -155,10 +156,9 @@ void missingRouteEntityLogsIssue() {
 
     var result = mapper.map(List.of(assignment));
 
-    assertTrue(result.isEmpty());
-    assertEquals(
-      List.of("NoticeAssignmentWithUnknownEntity"),
-      issues.listIssues().stream().map(DataImportIssue::getType).toList()
+    assertThat(result).isEmpty();
+    assertThat(issues.listIssues().stream().map(DataImportIssue::getType).toList()).containsExactly(
+      "NoticeAssignmentWithUnknownEntity"
     );
   }
 
@@ -167,13 +167,14 @@ void missingTripEntityLogsIssue() {
     var issues = new DefaultDataImportIssueStore();
     var noticeMapper = new NoticeMapper(ID_FACTORY);
     noticeMapper.map(GTFS_NOTICE);
+    var routeMapper = createRouteMapper();
 
     var mapper = new NoticeAssignmentMapper(
       ID_FACTORY,
       issues,
       noticeMapper,
-      createTripMapper(createRouteMapper()),
-      createRouteMapper()
+      createTripMapper(routeMapper),
+      routeMapper
     );
 
     var assignment = new NoticeAssignment();
@@ -183,10 +184,9 @@ void missingTripEntityLogsIssue() {
 
     var result = mapper.map(List.of(assignment));
 
-    assertTrue(result.isEmpty());
-    assertEquals(
-      List.of("NoticeAssignmentWithUnknownEntity"),
-      issues.listIssues().stream().map(DataImportIssue::getType).toList()
+    assertThat(result).isEmpty();
+    assertThat(issues.listIssues().stream().map(DataImportIssue::getType).toList()).containsExactly(
+      "NoticeAssignmentWithUnknownEntity"
     );
   }
 

application/src/test/java/org/opentripplanner/gtfs/mapping/NoticeMapperTest.java

@@ -1,6 +1,7 @@
 package org.opentripplanner.gtfs.mapping;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.onebusaway.gtfs.model.AgencyAndIdFactory.obaId;
 import static org.opentripplanner.transit.model._data.TimetableRepositoryForTest.FEED_ID;
@@ -34,7 +35,7 @@ void testMap() {
   @Test
   void testPublicCodeIsNull() {
     var result = subject.map(GTFS_NOTICE);
-    assertEquals(null, result.publicCode());
+    assertNull(result.publicCode());
   }
 
   @Test