Add Origin header support, switch to nmcp publishing, comprehensive Javadoc #9
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Use Gradle signing plugin directly with useInMemoryPgpKeys() - Read GPG_PRIVATE_KEY and GPG_PASSPHRASE from environment - Update workflows to pass correct env vars
- Replace vanniktech plugin with nmcp + maven-publish (matches tui4j) - Use SONATYPE_USERNAME/PASSWORD env vars directly - Update to actions/checkout@v6, gradle/actions/setup-gradle@v5 - Use publishAllPublicationsToCentralPortal tasks
Increment version from 0.1.2 to 0.1.3 across all project configuration files and documentation in preparation for the next release. - Update VERSION_NAME to 0.1.3-SNAPSHOT in gradle.properties - Update fallback version to 0.1.3 in build.gradle.kts - Update installation examples in README.md to reference 0.1.3
Add Javadoc documentation to all public classes, constructors, methods, and enum values to improve developer experience and enable proper API documentation generation. This includes the main client, gateway interface, domain models, request builders, and adapter layer. - Document AppleMaps client constructors and all public methods - Document AppleMapsGateway port interface methods - Add constructor Javadoc to domain records explaining parameter normalization - Document all PoiCategory, AddressCategory, and other enum values - Document request input builders and their fluent methods - Document adapter exceptions and their accessors - Document CLI entry point
Some Apple Maps JWT configurations require an Origin header that matches the origin claim in the token. This adds optional origin support throughout the SDK, allowing users to specify the origin via constructor parameter or environment variable. - Add origin parameter to AppleMaps constructors with overloads - Thread origin through HttpAppleMapsGateway to AppleMapsAuthorizationService - Set Origin header on token refresh and API requests when provided - Add getOrigin() accessor to AppleMapsAuthorizationService - Update CLI to read APPLE_MAPS_ORIGIN from env/system properties - Reorder imports to place java.* before project imports (style consistency)
Update documentation to explain how to configure the optional Origin header for JWT configurations that require it. - Add APPLE_MAPS_ORIGIN to environment variable examples - Update code example showing origin parameter usage - Document origin option in CLI prerequisites
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis release adds comprehensive origin header support for JWT authentication, refactors the publishing pipeline from Maven Central to Sonatype Central Portal, introduces builder patterns and query string serialization for request objects, and applies systematic normalization/validation to domain models throughout the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AppleMaps
participant HttpAppleMapsGateway
participant AuthService as AppleMapsAuthorizationService
participant HttpClient as HTTP Client
participant API as Apple Maps API
Client->>AppleMaps: AppleMaps(token, origin)
AppleMaps->>HttpAppleMapsGateway: HttpAppleMapsGateway(token, origin)
HttpAppleMapsGateway->>AuthService: AppleMapsAuthorizationService(token, timeout, origin)
AuthService->>AuthService: Store origin internally
Client->>AppleMaps: geocode(input)
AppleMaps->>HttpAppleMapsGateway: geocode(input)
HttpAppleMapsGateway->>AuthService: getOrigin()
AuthService-->>HttpAppleMapsGateway: Return origin value
HttpAppleMapsGateway->>HttpAppleMapsGateway: Build HTTP request<br/>conditionally add Origin header
HttpAppleMapsGateway->>HttpClient: POST with headers<br/>(Authorization, Origin)
HttpClient->>API: Request with Origin header
API-->>HttpClient: Response
HttpClient-->>HttpAppleMapsGateway: PlaceResults
HttpAppleMapsGateway-->>AppleMaps: PlaceResults
AppleMaps-->>Client: PlaceResults
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
WilliamAGH
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds support for the Origin HTTP header in Apple Maps Server API requests and includes extensive Javadoc documentation across the codebase. The Origin header is required for some JWT token configurations, making this an important enhancement for certain API usage scenarios.
Changes:
- Added optional
originparameter throughout the authorization and gateway layers to support Origin header in API requests - Added comprehensive Javadoc documentation to all public APIs, domain models, and builder classes
- Migrated from vanniktech maven-publish plugin to standard Gradle publishing with nmcp plugin for Maven Central deployment
- Updated version to 0.1.4-SNAPSHOT and adjusted documentation to reference 0.1.3
Reviewed changes
Copilot reviewed 54 out of 55 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/williamcallahan/applemaps/AppleMaps.java | Added constructors accepting optional origin parameter and comprehensive Javadoc |
| src/main/java/com/williamcallahan/applemaps/adapters/mapsserver/AppleMapsAuthorizationService.java | Added origin field and logic to include Origin header in token exchange requests |
| src/main/java/com/williamcallahan/applemaps/adapters/mapsserver/HttpAppleMapsGateway.java | Added origin parameter and logic to conditionally set Origin header on API requests |
| src/main/java/com/williamcallahan/applemaps/cli/AppleMapsCli.java | Added resolveOrigin() method to read APPLE_MAPS_ORIGIN environment variable |
| src/test/java/com/williamcallahan/applemaps/adapters/mapsserver/AppleMapsAuthorizationServiceTest.java | Updated tests to pass "origin" parameter in test dependencies |
| src/main/java/com/williamcallahan/applemaps/domain/request/*.java | Added Javadoc to constructors, builders, and public methods |
| src/main/java/com/williamcallahan/applemaps/domain/model/*.java | Added Javadoc to canonical constructors and enum values |
| src/main/java/com/williamcallahan/applemaps/domain/port/AppleMapsGateway.java | Added Javadoc to all interface methods |
| build.gradle.kts | Migrated from vanniktech plugin to standard Gradle publishing with nmcp plugin |
| gradle.properties | Updated version from 0.1.2-SNAPSHOT to 0.1.4-SNAPSHOT |
| docs/*.md | Updated documentation to reference Origin header configuration |
| .github/workflows/*.yaml | Updated environment variables for new signing approach |
| README.md | Updated version references from 0.1.2 to 0.1.3 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SONATYPE_PASSWORD: ${{ secrets.SONATYPE_PASSWORD }} | ||
| GPG_PRIVATE_KEY: ${{ secrets.GPG_PRIVATE_KEY }} | ||
| GPG_PASSPHRASE: ${{ secrets.GPG_PASSPHRASE }} | ||
| run: ./gradlew publishAllPublicationsToCentralPortalSnapshots -x test |
There was a problem hiding this comment.
The task name publishAllPublicationsToCentralPortalSnapshots used in the CI workflow does not exist. The nmcp plugin configuration only defines publishAllPublicationsToCentralPortal, which is meant for releases. For SNAPSHOT publishing, you should either use the standard Maven publishing task publishAllPublicationsToMavenRepository or configure a separate nmcp task for snapshots. The nmcp plugin's publishAllPublicationsToCentralPortal task is typically used for releases, not snapshots.
Suggested change
| run: ./gradlew publishAllPublicationsToCentralPortalSnapshots -x test | |
| run: ./gradlew publishAllPublicationsToMavenRepository -x test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New Features
originparameter toAppleMapsconstructors andAPPLE_MAPS_ORIGINenvironment variable support in CLIRefactoring
useInMemoryPgpKeys()for more reliable CI signingDocumentation
APPLE_MAPS_ORIGINconfiguration in authorization and CLI docsOther Changes