Avoid duplicate service IDs during GTFS merge

[JRroony]

Summary

• Reserve every raw ID originally present in the current source before allocating renamed collection IDs.
• Retry prefixed candidates until the ID is unused by both the merged output and the current source, then apply the final rename once so calendar, calendar-date, and trip references stay consistent.
• Add focused strategy tests and a writer/reader round-trip regression test for the T0 / a-T0 collision reported in Merging already merged feeds may create duplicate IDs #142.

Fixes #142

Testing

• mvn -pl onebusaway-gtfs-merge test (18 tests passed)
• mvn -pl onebusaway-gtfs-merge spotless:check
• git diff --check

Note: mvn -pl onebusaway-gtfs-merge -am test is blocked on Windows before reaching the merge module by two existing IndividualCsvEntityWriterTest LF-vs-CRLF assertions. The 14.2.3-SNAPSHOT dependencies were built with tests skipped, then the complete merge-module test suite was run successfully.

Summary by CodeRabbit

• Bug Fixes
  • Improved merging of service calendars when IDs conflict across source feeds.
  • Prevented renamed calendars from overwriting or taking IDs belonging to other source records.
  • Ensured related trips and exception dates continue referencing the correct calendars.
  • Added support for safely skipping multiple conflicting candidate IDs during renaming.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4571e48c-368a-476d-a397-6cbb922a8395

📥 Commits

Reviewing files that changed from the base of the PR and between c0ab195 and d08d29c.

📒 Files selected for processing (3)

• onebusaway-gtfs-merge/src/main/java/org/onebusaway/gtfs_merge/strategies/AbstractCollectionEntityMergeStrategy.java
• onebusaway-gtfs-merge/src/test/java/org/onebusaway/gtfs_merge/GtfsMergerTest.java
• onebusaway-gtfs-merge/src/test/java/org/onebusaway/gtfs_merge/strategies/ServiceCalendarMergeStrategyTest.java

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

The merge strategy now reserves source raw IDs and searches successive renamed candidates that do not collide with source or merged-output IDs. Tests cover service-calendar IDs, related references, fallback behavior, and chained collisions.

Changes

Service ID collision handling

Layer / File(s)	Summary
Collision-safe merge processing onebusaway-gtfs-merge/src/main/java/org/onebusaway/gtfs_merge/strategies/AbstractCollectionEntityMergeStrategy.java	The strategy snapshots source keys and raw IDs before processing. It rejects conflicting renamed candidates and throws when renaming does not change the raw ID.
Service-calendar regression coverage onebusaway-gtfs-merge/src/test/java/org/onebusaway/gtfs_merge/strategies/ServiceCalendarMergeStrategyTest.java, onebusaway-gtfs-merge/src/test/java/org/onebusaway/gtfs_merge/GtfsMergerTest.java	Tests cover simple and chained collisions, deterministic processing order, preserved entities, unique output IDs, and updated trip and calendar-date references.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to d08d2

The PR prevents duplicate service IDs during GTFS merging and adds focused regression coverage for reference consistency. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: preventing duplicate service IDs during GTFS merging.
Linked Issues check	✅ Passed	The implementation and regression tests address issue #142 by preventing duplicate calendar IDs and preserving related references.
Out of Scope Changes check	✅ Passed	The code and test changes remain focused on service ID collision handling described in issue #142.
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}