-
Notifications
You must be signed in to change notification settings - Fork 362
KTOR-9115 Re-work the Application structure topic #729
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…n structure topic
WalkthroughAdds a "Related topics" category entry; introduces a new routing-organization doc and updates navigation/tree entries to reference it; rewrites and expands server application structure and server modules documentation (architecture, DI, module loading/config); small edits to routing docs to point to the new routing file. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@topics/server-application-structure.md`:
- Around line 192-212: The diagram and example incorrectly place the
Ktor-specific CustomerRoutes.kt inside the domain layer; move CustomerRoutes.kt
out of domain into an application/presentation layer (e.g., application/ or
presentation/ package) and update the diagram so routes live under that layer,
not domain; also update the code example that references CustomerRoutes.kt (and
any imports) so it uses the new application/presentation package and ensure
domain files like Customer.kt and CustomerService.kt contain no Ktor imports or
route code.
In `@topics/server-routing-organization.md`:
- Around line 24-64: Add two aggregator extension functions named
customerRoutes() and orderRoutes() that group the existing route handlers:
implement Route.customerRoutes() to call customerByIdRoute() and
createCustomerRoute(), and implement Route.orderRoutes() to call getOrderRoute()
and totalizeOrderRoute(); ensure these aggregators are declared as Route
extension functions so the routing { customerRoutes(); orderRoutes() }
registration compiles and the individual handlers (customerByIdRoute,
createCustomerRoute, getOrderRoute, totalizeOrderRoute) are invoked inside each
aggregator.
| ```kotlin | ||
| fun Route.customerByIdRoute() { | ||
| get("/customer/{id}") { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| fun Route.createCustomerRoute() { | ||
| post("/customer") { | ||
|
|
||
| } | ||
| } | ||
| ``` | ||
| </tab> | ||
| <tab title="OrderRoutes.kt"> | ||
|
|
||
| ```kotlin | ||
| fun Route.getOrderRoute() { | ||
| get("/order/{id}") { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| fun Route.totalizeOrderRoute() { | ||
| get("/order/{id}/total") { | ||
|
|
||
| } | ||
| } | ||
| ``` | ||
| </tab> | ||
| </tabs> | ||
|
|
||
| Each file groups route handlers that belong to the same domain area. Then, you can register each group in your | ||
| routing block: | ||
|
|
||
| ```kotlin | ||
| routing { | ||
| customerRoutes() | ||
| orderRoutes() | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the file
fd topics/server-routing-organization.mdRepository: ktorio/ktor-documentation
Length of output: 51
🏁 Script executed:
# Check if the file exists and view lines 24-64
if [ -f "topics/server-routing-organization.md" ]; then
echo "=== File exists, showing lines 24-80 for context ==="
sed -n '24,80p' topics/server-routing-organization.md | cat -n
else
echo "File not found at topics/server-routing-organization.md"
# Try searching for similar files
find . -name "*server*routing*" -o -name "*routing*organization*" 2>/dev/null | head -20
fiRepository: ktorio/ktor-documentation
Length of output: 1531
🏁 Script executed:
# Let me check the full file to see if aggregators are defined elsewhere
wc -l topics/server-routing-organization.mdRepository: ktorio/ktor-documentation
Length of output: 110
🏁 Script executed:
# Check the beginning of the file and context around line 24
head -40 topics/server-routing-organization.md | cat -nRepository: ktorio/ktor-documentation
Length of output: 1449
🏁 Script executed:
# Check lines 36-70 to see if aggregators are defined after the individual route functions
sed -n '36,70p' topics/server-routing-organization.md | cat -nRepository: ktorio/ktor-documentation
Length of output: 1000
🏁 Script executed:
# Search the entire file for customerRoutes or orderRoutes definitions
grep -n "fun.*customerRoutes\|fun.*orderRoutes" topics/server-routing-organization.mdRepository: ktorio/ktor-documentation
Length of output: 154
🏁 Script executed:
# Let's examine the definitions of customerRoutes and orderRoutes at those lines
sed -n '95,115p' topics/server-routing-organization.md | cat -nRepository: ktorio/ktor-documentation
Length of output: 916
🏁 Script executed:
# Also check for orderRoutes definition
grep -n "fun.*orderRoutes" topics/server-routing-organization.mdRepository: ktorio/ktor-documentation
Length of output: 51
🏁 Script executed:
# Let me search for orderRoutes in the entire file
grep -n "orderRoutes" topics/server-routing-organization.mdRepository: ktorio/ktor-documentation
Length of output: 89
🏁 Script executed:
# Let's look at the full context around the "Group by file" section to understand what's shown
sed -n '13,65p' topics/server-routing-organization.md | cat -nRepository: ktorio/ktor-documentation
Length of output: 1372
🏁 Script executed:
# Let's confirm the full extent of the "Group by file" section to be thorough
sed -n '13,80p' topics/server-routing-organization.md | cat -nRepository: ktorio/ktor-documentation
Length of output: 1939
Add aggregator functions to complete the "Group by file" example.
The example defines individual route handlers but the registration code calls customerRoutes() and orderRoutes() which aren't shown. The example needs these aggregators to be complete and compilable.
✏️ Proposed fix (add aggregators)
fun Route.createCustomerRoute() {
post("/customer") {
}
}
+
+fun Route.customerRoutes() {
+ customerByIdRoute()
+ createCustomerRoute()
+} fun Route.totalizeOrderRoute() {
get("/order/{id}/total") {
}
}
+
+fun Route.orderRoutes() {
+ getOrderRoute()
+ totalizeOrderRoute()
+}🤖 Prompt for AI Agents
In `@topics/server-routing-organization.md` around lines 24 - 64, Add two
aggregator extension functions named customerRoutes() and orderRoutes() that
group the existing route handlers: implement Route.customerRoutes() to call
customerByIdRoute() and createCustomerRoute(), and implement Route.orderRoutes()
to call getOrderRoute() and totalizeOrderRoute(); ensure these aggregators are
declared as Route extension functions so the routing { customerRoutes();
orderRoutes() } registration compiles and the individual handlers
(customerByIdRoute, createCustomerRoute, getOrderRoute, totalizeOrderRoute) are
invoked inside each aggregator.
| In this structure, each service owns an isolated domain slice and remains modular internally, integrating with service | ||
| discovery, metrics, and external configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also indicate here that you can use the canned main functions (i.e., io.ktor.server.cio.EngineMain) instead of the entry points (Application.kt), and you can deploy multiple microservices in the same instance via configuration, which is commonly referred to as a "modulith" deployment these days.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added 2 sub-sections mentioning these. Can you check if they are correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I meant like the gradle modules for the microservices can be packaged independently and deployed together in the same application. It's not very important, but this modulith approach is getting more popular for teams that want the independence but without the overhead of managing multiple services.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@topics/server-application-structure.md`:
- Around line 52-53: The link target uses the fragment
`#microservice-oriented-structure` but the corresponding heading "Microservice
architectures" is missing an explicit id; update that heading by appending the
explicit id {`#microservice-oriented-structure`} to the "Microservice
architectures" heading so the link fragment resolves and MD051 is satisfied.
- Around line 26-38: The fenced directory-tree code blocks in
topics/server-application-structure.md are missing a language tag (triggering
MD040); update each triple-backtick fence that contains a project/ directory
tree to use a plain language like ```text so they are treated as text.
Specifically, change the fence around the shown tree (the block that starts with
"project/" and contains "src/ main/ kotlin/ Application.kt") and apply the same
replacement to the other directory-tree fences referenced (the blocks at the
other ranges listed) so every directory-tree fence uses ```text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@topics/server-application-structure.md`:
- Around line 246-256: The snippet's CustomerService lacks a findById used by
routes; add a public suspend fun findById(id: CustomerId): Customer? to
CustomerService that delegates to repository.find(id) (or return
repository.find(id)?.let { CustomerAggregate(it) } depending on intended
return), so callers to customerService.findById(...) compile; update both
occurrences (lines near addContact and the other instance) to use this new
method if needed.
| class CustomerService( | ||
| private val repository: CustomerRepository, | ||
| private val events: EventPublisher | ||
| ) { | ||
| suspend fun addContact(id: CustomerId, contact: Contact): Customer? { | ||
| val customer = repository.find(id) ?: return null | ||
| val updated = CustomerAggregate(customer).addContact(contact) | ||
| repository.save(updated) | ||
| events.publish(CustomerContactAdded(id, contact)) | ||
| return updated | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep the DDD snippets consistent (missing findById).
CustomerService only defines addContact, but the route calls customerService.findById(...). Add the method (or change the route) so the example compiles as written.
✅ Proposed fix (add method to the snippet)
class CustomerService(
private val repository: CustomerRepository,
private val events: EventPublisher
) {
suspend fun addContact(id: CustomerId, contact: Contact): Customer? {
val customer = repository.find(id) ?: return null
val updated = CustomerAggregate(customer).addContact(contact)
repository.save(updated)
events.publish(CustomerContactAdded(id, contact))
return updated
}
+
+ suspend fun findById(id: CustomerId): Customer? =
+ repository.find(id)
}Also applies to: 287-291
🤖 Prompt for AI Agents
In `@topics/server-application-structure.md` around lines 246 - 256, The snippet's
CustomerService lacks a findById used by routes; add a public suspend fun
findById(id: CustomerId): Customer? to CustomerService that delegates to
repository.find(id) (or return repository.find(id)?.let { CustomerAggregate(it)
} depending on intended return), so callers to customerService.findById(...)
compile; update both occurrences (lines near addContact and the other instance)
to use this new method if needed.
KTOR-9115