Driver code done by using Smithy generated code#9
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces the Spring Boot application with a Smithy-generated Java server: updates build and settings to use Smithy, adds Changes
Sequence DiagramsequenceDiagram
participant main as main()
participant RS as RouteServer
participant Builder as RouteService.Builder
participant CA as CreateRouteActivity
participant DA as DeleteRouteActivity
participant GA as GetRouteActivity
participant Server as Server
main->>RS: new RouteServer()
main->>RS: run()
RS->>Builder: new RouteService.Builder()
Builder->>CA: instantiate & register CreateRouteActivity
Builder->>DA: instantiate & register DeleteRouteActivity
Builder->>GA: instantiate & register GetRouteActivity
Builder->>Server: build() -> create server(endpoint)
RS->>Server: start()
RS->>RS: block (join thread)
Note over RS,Server: on interrupt
RS->>Server: shutdown()
RS->>Server: awaitTermination()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
build.gradle.kts (1)
82-89:⚠️ Potential issue | 🟡 MinorPlatform-specific command limits portability.
The
smithyBuildtask usescmd /cwhich only works on Windows. Consider using a cross-platform approach or the Smithy Gradle plugin for better portability.♻️ Suggested cross-platform alternative
tasks.register<Exec>("smithyBuild") { group = "build" description = "Java models codegen from Smithy model." - commandLine("cmd", "/c", "smithy", "build", + commandLine("smithy", "build", "--config", ".\\InAndOut-API-Modelling\\smithy-build.json", "--projection", "java-route-service", "--output", "build/") + if (System.getProperty("os.name").lowercase().contains("windows")) { + commandLine = listOf("cmd", "/c") + commandLine + } }Alternatively, consider using the official Smithy Gradle plugin (
software.amazon.smithy.gradle.smithy-baseorsoftware.amazon.smithy.gradle.smithy-jar) which handles this natively.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle.kts` around lines 82 - 89, The smithyBuild task is Windows-only because it calls "cmd /c"; change it to a cross-platform implementation by either (A) switching the task to invoke the smithy CLI directly (remove "cmd", " /c" and call commandLine("smithy", ...") or use exec { executable = "smithy"; args = [...] } and add an OS-guard if you must support different executables on Windows vs *nix, or (B) preferably apply the official Smithy Gradle plugin (software.amazon.smithy.gradle.smithy-base or software.amazon.smithy.gradle.smithy-jar) and replace the custom tasks with the plugin's smithy configuration and its built-in tasks (refer to the smithyBuild task name and configure the plugin's projection "java-route-service" and output to "build/").
🧹 Nitpick comments (2)
build.gradle.kts (1)
47-67: Consider removing unused Spring Boot dependencies.The PR transitions from Spring Boot to a Smithy-based HTTP server, yet the Spring Boot plugins (lines 4-6) and all Spring Boot starter dependencies remain. If these are no longer used, they add unnecessary bloat and potential classpath conflicts.
If this is an incremental migration, consider documenting a TODO to remove these dependencies once the transition is complete. Otherwise, remove them now.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle.kts` around lines 47 - 67, The build includes many Spring Boot starter dependencies (e.g., implementation("org.springframework.boot:spring-boot-starter-jdbc"), implementation("org.springframework.boot:spring-boot-starter-r2dbc"), implementation("org.springframework.boot:spring-boot-starter-restclient"), implementation("org.springframework.boot:spring-boot-starter-security"), implementation("org.springframework.boot:spring-boot-starter-security-oauth2-authorization-server"), implementation("org.springframework.boot:spring-boot-starter-security-oauth2-client"), implementation("org.springframework.boot:spring-boot-starter-session-jdbc"), implementation("org.springframework.boot:spring-boot-starter-webclient"), implementation("org.springframework.boot:spring-boot-starter-webmvc"), implementation("org.springframework.boot:spring-boot-starter-webservices") and corresponding testImplementation entries) that are likely unused after migrating to the Smithy HTTP server; either remove these implementation/testImplementation lines now (after verifying no runtime/test usage) or keep them temporarily but add a clear TODO comment in build.gradle.kts indicating they should be removed once the Spring Boot pieces are fully migrated to avoid classpath bloat and conflicts.src/main/java/com/shopping/inandout/routeservice/activities/CreateRouteActivity.java (1)
9-9: Remove the personal comment from committed source.This line is unrelated to the handler and will just add noise to future diffs. Please keep production files free of personal comments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/shopping/inandout/routeservice/activities/CreateRouteActivity.java` at line 9, Remove the unrelated personal comment in CreateRouteActivity.java: delete the stray line "// i love my girlfriend <333333333333" inside the CreateRouteActivity class/file so the source contains only production-relevant comments and code; ensure no other personal or nonfunctional comments remain in the CreateRouteActivity class header or methods (e.g., constructor, handleRequest).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build.gradle.kts`:
- Around line 74-80: The java.srcDir currently points into the package path
which breaks compilation; update the afterEvaluate -> sourceSets -> main
configuration so that
java.srcDir("build/java-route-service/java-server-codegen") (i.e. the generated
sources root) is used instead of the full package path; modify the java.srcDir
call in the afterEvaluate block that configures sourceSets.main to reference the
generated-source root rather than ".../com/shopping/inandout".
In
`@src/main/java/com/shopping/inandout/routeservice/activities/CreateRouteActivity.java`:
- Around line 12-13: The createRoute method in CreateRouteActivity currently
ignores the input and returns an empty CreateRouteOutput; update
CreateRouteActivity.createRoute to either call the real creation flow (e.g.,
delegate to your route persistence/service method such as
RouteService.createRoute or RouteRepository.save using the provided
CreateRouteInput and RequestContext) and return a populated CreateRouteOutput
reflecting the persisted route, or if the implementation isn’t ready, explicitly
fail by throwing a clear exception (e.g., UnsupportedOperationException) instead
of returning a success; ensure the method uses CreateRouteInput fields to build
the created resource and include any IDs or status in CreateRouteOutput.
In
`@src/main/java/com/shopping/inandout/routeservice/activities/DeleteRouteActivity.java`:
- Around line 10-11: deleteRoute currently always returns success without doing
any deletion; update DeleteRouteActivity.deleteRoute to invoke the actual
deletion flow using the route identifier from DeleteRouteInput (e.g., call the
repository or service delete method such as RouteRepository.deleteById or
RouteService.deleteRoute), handle not-found cases by returning a
failure/exception instead of success, and surface errors/logging via the
provided RequestContext; ensure DeleteRouteOutput reflects the real outcome
(success only when delete succeeded).
In
`@src/main/java/com/shopping/inandout/routeservice/activities/GetRouteActivity.java`:
- Around line 10-11: getRoute currently always returns an empty GetRouteOutput
regardless of input; update GetRouteActivity.getRoute to use the provided
GetRouteInput and RequestContext to perform the actual lookup (e.g., call your
route repository/service with input identifiers) and populate the GetRouteOutput
with the found route data, and if no route is found return or throw a modeled
not-found error (or a not-implemented error if lookup is not yet available)
instead of returning a successful empty body.
In `@src/main/java/com/shopping/inandout/routeservice/RouteService.java`:
- Around line 31-34: The Server is hard-coded to bind to "http://localhost:8888"
in RouteService (Server.builder().endpoints(...)), preventing external/container
access; change it to read the bind address and port from configuration/env
variables (e.g., System.getenv("BIND_ADDR") and System.getenv("PORT") or your
app config provider) and fall back to a non-loopback default like "0.0.0.0" and
port 8888; construct the endpoint URI from those values
(URI.create(String.format("http://%s:%s", bindAddr, port))) and pass that to
Server.builder().endpoints(...) so the service is configurable at deploy time.
- Around line 39-48: Replace the blocking Thread.currentThread().join() approach
and move the graceful shutdown logic into a Runtime.addShutdownHook so JVM
signals trigger server.shutdown().get(); keep the main thread waiting with a
CountDownLatch or CompletableFuture that the shutdown hook completes, and in any
InterruptedException handling (around join or waiting) restore the interrupt
flag (Thread.currentThread().interrupt()) before returning; update places
referencing Thread.currentThread().join(), the catch of InterruptedException,
LOGGER.info("Stopping server...") and server.shutdown().get() to use this
shutdown hook + latch/future pattern so resources are cleaned on JVM
termination.
---
Outside diff comments:
In `@build.gradle.kts`:
- Around line 82-89: The smithyBuild task is Windows-only because it calls "cmd
/c"; change it to a cross-platform implementation by either (A) switching the
task to invoke the smithy CLI directly (remove "cmd", " /c" and call
commandLine("smithy", ...") or use exec { executable = "smithy"; args = [...] }
and add an OS-guard if you must support different executables on Windows vs
*nix, or (B) preferably apply the official Smithy Gradle plugin
(software.amazon.smithy.gradle.smithy-base or
software.amazon.smithy.gradle.smithy-jar) and replace the custom tasks with the
plugin's smithy configuration and its built-in tasks (refer to the smithyBuild
task name and configure the plugin's projection "java-route-service" and output
to "build/").
---
Nitpick comments:
In `@build.gradle.kts`:
- Around line 47-67: The build includes many Spring Boot starter dependencies
(e.g., implementation("org.springframework.boot:spring-boot-starter-jdbc"),
implementation("org.springframework.boot:spring-boot-starter-r2dbc"),
implementation("org.springframework.boot:spring-boot-starter-restclient"),
implementation("org.springframework.boot:spring-boot-starter-security"),
implementation("org.springframework.boot:spring-boot-starter-security-oauth2-authorization-server"),
implementation("org.springframework.boot:spring-boot-starter-security-oauth2-client"),
implementation("org.springframework.boot:spring-boot-starter-session-jdbc"),
implementation("org.springframework.boot:spring-boot-starter-webclient"),
implementation("org.springframework.boot:spring-boot-starter-webmvc"),
implementation("org.springframework.boot:spring-boot-starter-webservices") and
corresponding testImplementation entries) that are likely unused after migrating
to the Smithy HTTP server; either remove these implementation/testImplementation
lines now (after verifying no runtime/test usage) or keep them temporarily but
add a clear TODO comment in build.gradle.kts indicating they should be removed
once the Spring Boot pieces are fully migrated to avoid classpath bloat and
conflicts.
In
`@src/main/java/com/shopping/inandout/routeservice/activities/CreateRouteActivity.java`:
- Line 9: Remove the unrelated personal comment in CreateRouteActivity.java:
delete the stray line "// i love my girlfriend <333333333333" inside the
CreateRouteActivity class/file so the source contains only production-relevant
comments and code; ensure no other personal or nonfunctional comments remain in
the CreateRouteActivity class header or methods (e.g., constructor,
handleRequest).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 57d41dad-6eb0-4624-8a3c-c48a6e055970
📒 Files selected for processing (7)
InAndOut-API-Modellingbuild.gradle.ktssrc/main/java/com/shopping/inandout/routeservice/RouteService.javasrc/main/java/com/shopping/inandout/routeservice/RouteServiceApplication.javasrc/main/java/com/shopping/inandout/routeservice/activities/CreateRouteActivity.javasrc/main/java/com/shopping/inandout/routeservice/activities/DeleteRouteActivity.javasrc/main/java/com/shopping/inandout/routeservice/activities/GetRouteActivity.java
💤 Files with no reviewable changes (1)
- src/main/java/com/shopping/inandout/routeservice/RouteServiceApplication.java
src/main/java/com/shopping/inandout/routeservice/activities/CreateRouteActivity.java
Outdated
Show resolved
Hide resolved
src/main/java/com/shopping/inandout/routeservice/activities/DeleteRouteActivity.java
Outdated
Show resolved
Hide resolved
src/main/java/com/shopping/inandout/routeservice/activities/GetRouteActivity.java
Outdated
Show resolved
Hide resolved
src/main/java/com/shopping/inandout/routeservice/RouteServiceWrapper.java
Show resolved
Hide resolved
src/main/java/com/shopping/inandout/routeservice/RouteServiceWrapper.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/test/java/com/shopping/inandout/routeservice/RouteServerTest.java (1)
5-11: Empty test provides no value.The
contextLoads()test was previously a Spring Boot context verification. Now that Spring Boot is removed, this test does nothing. Consider either implementing actual unit tests forRouteServer(e.g., verifying service registration, endpoint configuration) or removing this placeholder.♻️ Option 1: Remove placeholder test
-class RouteServerTest { - - `@Test` - void contextLoads() { - } - -} +// TODO: Add meaningful tests for RouteServer♻️ Option 2: Add basic smoke test
class RouteServerTest { `@Test` - void contextLoads() { + void serverCanBeInstantiated() { + RouteServer server = new RouteServer(); + // Note: full integration test would require starting/stopping the server + assertNotNull(server); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/shopping/inandout/routeservice/RouteServerTest.java` around lines 5 - 11, The placeholder test method contextLoads() in class RouteServerTest is empty and should be removed or replaced with a real unit test; either delete RouteServerTest entirely if no smoke test is needed, or replace contextLoads() with a focused test that instantiates RouteServer (or its constructor/factory), exercises a public behavior (e.g., call start()/initialize() or registerService()/getEndpoints()), and asserts expected outcomes (service registered, expected endpoints present, or no exceptions thrown) using your test framework's assertions—locate RouteServerTest and the RouteServer class to implement the chosen change.build.gradle.kts (2)
39-43: Extract Lombok version to a variable to reduce duplication.The Lombok version
1.18.30is repeated four times. Consider extracting it to a variable for easier maintenance.♻️ Proposed refactor
dependencies { val smithyJavaVersion: String by project + val lombokVersion = "1.18.30" // Smithy java model generation smithyBuild("software.amazon.smithy.java.codegen:plugins:$smithyJavaVersion") implementation(project(":InAndOut-API-Modelling")) // Adds an HTTP server implementation based on netty implementation("software.amazon.smithy.java:server-netty:$smithyJavaVersion") // Adds the server implementation of the `RestJson1` protocol implementation("software.amazon.smithy.java:aws-server-restjson:$smithyJavaVersion") - compileOnly("org.projectlombok:lombok:1.18.30") - annotationProcessor("org.projectlombok:lombok:1.18.30") + compileOnly("org.projectlombok:lombok:$lombokVersion") + annotationProcessor("org.projectlombok:lombok:$lombokVersion") - testCompileOnly("org.projectlombok:lombok:1.18.30") - testAnnotationProcessor("org.projectlombok:lombok:1.18.30") + testCompileOnly("org.projectlombok:lombok:$lombokVersion") + testAnnotationProcessor("org.projectlombok:lombok:$lombokVersion")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle.kts` around lines 39 - 43, Extract the repeated Lombok version string into a single variable (e.g., lombokVersion) and replace the four hardcoded occurrences used by compileOnly, annotationProcessor, testCompileOnly, and testAnnotationProcessor with references to that variable; define the variable in the build.gradle.kts scope above the dependencies block and use it for all four dependency declarations so future upgrades require changing the version in only one place.
45-46: Consider updating JUnit to a more recent version.JUnit Jupiter 5.7.1 is from January 2021. Consider updating to a more recent version (5.10.x or later) for bug fixes and new features, unless there's a specific compatibility requirement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle.kts` around lines 45 - 46, Replace the outdated JUnit dependency version string testImplementation("org.junit.jupiter:junit-jupiter:5.7.1") with a current stable release (for example 5.10.x or later), and update any related test runtime dependency versions if needed (e.g., testRuntimeOnly("org.junit.platform:junit-platform-launcher")) so they remain compatible; after updating the version coordinates, run the test suite/build to verify compatibility and adjust plugin or Gradle settings if any deprecation warnings appear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gradle/wrapper/gradle-wrapper.properties`:
- Line 3: Update the gradle wrapper properties to restore wrapper integrity
validation and document the downgrade rationale: add or re-enable the
validateDistributionUrl=true property alongside the existing distributionUrl
(distributionUrl=https\://services.gradle.org/distributions/gradle-8.2.1-bin.zip)
and include a brief comment explaining why you downgraded from 9.3.1 to 8.2.1
(e.g., specific build/tooling/CI compatibility or a blocker), or revert to 9.3.1
if no concrete justification exists; ensure the validateDistributionUrl entry is
present to maintain security validation and that the comment references the
distributionUrl version.
In `@settings.gradle.kts`:
- Around line 17-18: The build includes the subproject
include("InAndOut-API-Modelling") which is a git submodule (see .gitmodules) and
will cause fresh clones to fail; update the README to add a short “Setup” note
telling developers to initialize submodules after cloning by either cloning with
submodules or running the submodule init/update command (mention the standard
git commands to clone with --recurse-submodules or to run git submodule update
--init --recursive) so the InAndOut-API-Modelling subproject is populated before
running Gradle.
---
Nitpick comments:
In `@build.gradle.kts`:
- Around line 39-43: Extract the repeated Lombok version string into a single
variable (e.g., lombokVersion) and replace the four hardcoded occurrences used
by compileOnly, annotationProcessor, testCompileOnly, and
testAnnotationProcessor with references to that variable; define the variable in
the build.gradle.kts scope above the dependencies block and use it for all four
dependency declarations so future upgrades require changing the version in only
one place.
- Around line 45-46: Replace the outdated JUnit dependency version string
testImplementation("org.junit.jupiter:junit-jupiter:5.7.1") with a current
stable release (for example 5.10.x or later), and update any related test
runtime dependency versions if needed (e.g.,
testRuntimeOnly("org.junit.platform:junit-platform-launcher")) so they remain
compatible; after updating the version coordinates, run the test suite/build to
verify compatibility and adjust plugin or Gradle settings if any deprecation
warnings appear.
In `@src/test/java/com/shopping/inandout/routeservice/RouteServerTest.java`:
- Around line 5-11: The placeholder test method contextLoads() in class
RouteServerTest is empty and should be removed or replaced with a real unit
test; either delete RouteServerTest entirely if no smoke test is needed, or
replace contextLoads() with a focused test that instantiates RouteServer (or its
constructor/factory), exercises a public behavior (e.g., call
start()/initialize() or registerService()/getEndpoints()), and asserts expected
outcomes (service registered, expected endpoints present, or no exceptions
thrown) using your test framework's assertions—locate RouteServerTest and the
RouteServer class to implement the chosen change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc6db027-f0c1-4bcf-ae83-9c2c961188fc
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (11)
.env.exampleInAndOut-API-Modellingbuild.gradle.ktsgradle/wrapper/gradle-wrapper.propertiesgradlewgradlew.batsettings.gradle.ktssmithy-build.jsonsrc/main/java/com/shopping/inandout/routeservice/RouteServer.javasrc/main/resources/application.propertiessrc/test/java/com/shopping/inandout/routeservice/RouteServerTest.java
💤 Files with no reviewable changes (1)
- src/main/resources/application.properties
✅ Files skipped from review due to trivial changes (2)
- smithy-build.json
- InAndOut-API-Modelling
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/shopping/inandout/routeservice/activities/DeleteRouteActivity.java`:
- Around line 10-11: Replace the placeholder exception in
DeleteRouteActivity.deleteRoute with the actual delete flow: use the
DeleteRouteInput to extract the route id, call the backing repository/service
(e.g., routeRepository.deleteById(id) or routeService.deleteRoute(id)) to
perform deletion, check the result and return a populated DeleteRouteOutput on
success, and map not-found or failure cases to explicit service errors (e.g.,
return a 404-style service error when the id is missing/not found and a
meaningful error response for other failures) instead of throwing
UnsupportedOperationException; ensure you use RequestContext for
logging/authorization as needed and bubble up/translate repository exceptions
into well-defined service exceptions rather than allowing runtime exceptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca8e5f03-c62e-485d-9a1e-ecd9b4381ac0
📒 Files selected for processing (3)
src/main/java/com/shopping/inandout/routeservice/activities/CreateRouteActivity.javasrc/main/java/com/shopping/inandout/routeservice/activities/DeleteRouteActivity.javasrc/main/java/com/shopping/inandout/routeservice/activities/GetRouteActivity.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/shopping/inandout/routeservice/activities/GetRouteActivity.java
- src/main/java/com/shopping/inandout/routeservice/activities/CreateRouteActivity.java
src/main/java/com/shopping/inandout/routeservice/activities/DeleteRouteActivity.java
Show resolved
Hide resolved
|
|
||
| rootProject.name = "routeservice" | ||
| // Subprojects | ||
| include("InAndOut-API-Modelling") |
There was a problem hiding this comment.
maybe we should keep the app as a subproject too. See
https://github.com/smithy-lang/smithy-examples/blob/main/smithy-java-examples/quickstart-java/settings.gradle.kts
|
|
||
| String endpoint = System.getenv() | ||
| .getOrDefault("ROUTE_SERVICE_ENDPOINT", "http://0.0.0.0:8888"); | ||
| Server server = Server.builder() |
There was a problem hiding this comment.
maybe this can be cool https://dagger.dev/dev-guide/
|



The goal of this PR is to be able to build the project and run it locally. The expected functional output is the ability to respond with 200 to simple curl commands in the terminal.
Summary by CodeRabbit
New Features
Configuration
Behavior
Tests
Chores