Feature: #182, YAML Config File Generation #211
Conversation
cchunduri
changed the title
|
Hi @cchunduri , very good work on providing PR. only few minor comments needs to be addressed. Can you also update the ReadMe file with this change. |
|
@rajadilipkolli Thank you for your review. In README.md, do I need to update features section? Could please elaborate? |
|
Hi @cchunduri |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughAdds a new prompt to choose configuration file format, updates server generator to dynamically select resource and test templates based on that choice, and introduces YAML templates for application, local, and test configurations with conditional content (DB vendor, Flyway, ELK, LocalStack). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant P as Prompts
participant G as Server Generator
participant R as Template Resolver
participant FS as File Writer
U->>P: Answer prompts (incl. propFileFormat)
P-->>G: configOptions { propFileFormat, ... }
G->>R: get resource templates(configOptions)
R-->>G: application[.yml|.properties], application-local[.yml|.properties], logback-*.xml
G->>R: get test templates(configOptions)
R-->>G: application-test[.yml|.properties], logback-test.xml
G->>FS: Generate files from selected templates
FS-->>G: Write complete
G-->>U: Generation finished
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
generators/server/prompts.js (1)
127-141: Polish the prompt: capitalization + sensible default; keep terminology consistent.
- Capitalize the message for consistency with other prompts.
- Add a default to avoid undefined behavior in non‑interactive runs. Choose based on desired backward compatibility (props) vs. new feature (yaml).
Proposed diff (defaulting to properties for backward compatibility):
{ type: 'list', name: 'propFileFormat', - message: 'which format do you want to use for configuration files?', + message: 'Which format do you want to use for configuration files?', choices: [ { value: 'props', - name: 'Property File' + name: 'Properties' }, { value: 'yaml', name: 'YAML' } - ] + ], + default: 'props' }generators/server/templates/app/src/test/resources/application-test.yml (1)
1-11: Verify Spring Cloud AWS endpoint properties; consider service‑specific endpoints.Depending on the Spring Cloud AWS version, a single
cloud.aws.endpointmay be ignored. Service‑specific endpoints (sqs.endpoint,sns.endpoint,s3.endpoint) are more robust for LocalStack.Would you confirm the Spring Cloud AWS version used by the generated project and adjust if needed? If on 3.x+, prefer:
spring: cloud: aws: region: static: us-east-1 credentials: access-key: noop secret-key: noop sqs: endpoint: http://localhost:4566 sns: endpoint: http://localhost:4566 s3: endpoint: http://localhost:4566generators/server/index.js (3)
219-229: Guard against unexpected values; simplify naming.
- Treat any non‑
yamlinput aspropsto avoid surprises.- Rename
testResYamlTemplates→testResTemplatesfor clarity._getResourceFileTestTemplates(configOptions) { - let testResYamlTemplates = [] - - if (configOptions.propFileFormat === 'yaml') { - testResYamlTemplates = ['application-test.yml'] - } - else { - testResYamlTemplates = ['application-test.properties'] - } - return [...testResYamlTemplates, 'logback-test.xml'] + const format = configOptions.propFileFormat === 'yaml' ? 'yaml' : 'props'; + const testResTemplates = format === 'yaml' + ? ['application-test.yml'] + : ['application-test.properties']; + return [...testResTemplates, 'logback-test.xml']; }
231-241: Mirror the same defensive logic for main resources._getResourceFileTemplates(configOptions) { - let mainResTemplates = [] - - if (configOptions.propFileFormat === 'yaml') { - mainResTemplates = ['application.yml', 'application-local.yml'] - } else { - mainResTemplates = ['application.properties', 'application-local.properties'] - } - - return [...mainResTemplates, 'logback-spring.xml'] + const format = configOptions.propFileFormat === 'yaml' ? 'yaml' : 'props'; + const mainResTemplates = format === 'yaml' + ? ['application.yml', 'application-local.yml'] + : ['application.properties', 'application-local.properties']; + return [...mainResTemplates, 'logback-spring.xml']; }
202-213: Add the two config bootstrap tests requested in PR comments.One Maven and one Gradle IT to assert app context starts with both formats would de‑risk this change.
I can scaffold two minimal integration tests (yaml and properties) and wire them into both Maven Surefire/Failsafe and Gradle test tasks. Want me to push a patch?
generators/server/templates/app/src/main/resources/application.yml (2)
11-14: Use native YAML list for actuator exposure.Comma‑separated string works but YAML list is clearer.
server: - port: '8080' + port: 8080 shutdown: graceful @@ endpoints: web: exposure: - include: configprops,env,health,info,logfile,loggers,metrics,prometheus + include: + - configprops + - env + - health + - info + - logfile + - loggers + - metrics + - prometheus
63-66: Optional: keep customapplication.*block or prefix with your app namespace.Top‑level
application:may be confused withspring.application. Consider namespacing (e.g.,app:).-application: - logstash-host: localhost +app: + logstash-host: localhostIf existing code reads
application.logstash-host, keep it as‑is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
generators/server/index.js(2 hunks)generators/server/prompts.js(1 hunks)generators/server/templates/app/src/main/resources/application-local.yml(1 hunks)generators/server/templates/app/src/main/resources/application.yml(1 hunks)generators/server/templates/app/src/test/resources/application-test.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
generators/server/templates/app/src/main/resources/application.yml
[error] 26-26: duplication of key "spring" in mapping
(key-duplicates)
[error] 59-59: syntax error: could not find expected ':'
(syntax)
generators/server/templates/app/src/test/resources/application-test.yml
[error] 2-2: syntax error: mapping values are not allowed here
(syntax)
generators/server/templates/app/src/main/resources/application-local.yml
[error] 2-2: syntax error: mapping values are not allowed here
(syntax)
🔇 Additional comments (2)
generators/server/index.js (2)
198-201: Good extraction of main resource selection.Helper improves readability and centralizes format branching.
214-216: Good extraction of test resource selection.Consistent with main resource helper.
| <%_ if (databaseType === 'postgresql') { _%> | ||
| spring: | ||
| datasource: | ||
| driver-class-name: org.postgresql.Driver | ||
| url: jdbc:postgresql://localhost:5432/appdb | ||
| <%_ } _%> | ||
| <%_ if (databaseType === 'mysql') { _%> | ||
| datasource: | ||
| driver-class-name: com.mysql.jdbc.Driver | ||
| url: jdbc:mysql://localhost:3306/appdb | ||
| <%_ } _%> | ||
| <%_ if (databaseType === 'mariadb') { _%> | ||
| datasource: | ||
| driver-class-name: org.mariadb.jdbc.Driver | ||
| url: jdbc:mariadb://localhost:3306/appdb | ||
| <%_ } _%> | ||
| datasource: | ||
| username: appuser | ||
| password: secret | ||
|
|
There was a problem hiding this comment.
Broken YAML for non‑PostgreSQL paths; spring: missing for MySQL/MariaDB; misplaced username/password.
- When
databaseTypeismysqlormariadb,spring:is not emitted, producing invalid YAML. datasource.username/passwordare outsidespring:for those branches.
Apply this consolidated structure (works for all DBs):
-<%_ if (databaseType === 'postgresql') { _%>
-spring:
- datasource:
- driver-class-name: org.postgresql.Driver
- url: jdbc:postgresql://localhost:5432/appdb
-<%_ } _%>
-<%_ if (databaseType === 'mysql') { _%>
- datasource:
- driver-class-name: com.mysql.jdbc.Driver
- url: jdbc:mysql://localhost:3306/appdb
-<%_ } _%>
-<%_ if (databaseType === 'mariadb') { _%>
- datasource:
- driver-class-name: org.mariadb.jdbc.Driver
- url: jdbc:mariadb://localhost:3306/appdb
-<%_ } _%>
- datasource:
- username: appuser
- password: secret
+spring:
+ datasource:
+ <%_ if (databaseType === 'postgresql') { _%>
+ driver-class-name: org.postgresql.Driver
+ url: jdbc:postgresql://localhost:5432/appdb
+ <%_ } _%>
+ <%_ if (databaseType === 'mysql') { _%>
+ driver-class-name: com.mysql.cj.jdbc.Driver
+ url: jdbc:mysql://localhost:3306/appdb
+ <%_ } _%>
+ <%_ if (databaseType === 'mariadb') { _%>
+ driver-class-name: org.mariadb.jdbc.Driver
+ url: jdbc:mariadb://localhost:3306/appdb
+ <%_ } _%>
+ username: appuser
+ password: secret📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <%_ if (databaseType === 'postgresql') { _%> | |
| spring: | |
| datasource: | |
| driver-class-name: org.postgresql.Driver | |
| url: jdbc:postgresql://localhost:5432/appdb | |
| <%_ } _%> | |
| <%_ if (databaseType === 'mysql') { _%> | |
| datasource: | |
| driver-class-name: com.mysql.jdbc.Driver | |
| url: jdbc:mysql://localhost:3306/appdb | |
| <%_ } _%> | |
| <%_ if (databaseType === 'mariadb') { _%> | |
| datasource: | |
| driver-class-name: org.mariadb.jdbc.Driver | |
| url: jdbc:mariadb://localhost:3306/appdb | |
| <%_ } _%> | |
| datasource: | |
| username: appuser | |
| password: secret | |
| spring: | |
| datasource: | |
| <%_ if (databaseType === 'postgresql') { _%> | |
| driver-class-name: org.postgresql.Driver | |
| url: jdbc:postgresql://localhost:5432/appdb | |
| <%_ } _%> | |
| <%_ if (databaseType === 'mysql') { _%> | |
| driver-class-name: com.mysql.cj.jdbc.Driver | |
| url: jdbc:mysql://localhost:3306/appdb | |
| <%_ } _%> | |
| <%_ if (databaseType === 'mariadb') { _%> | |
| driver-class-name: org.mariadb.jdbc.Driver | |
| url: jdbc:mariadb://localhost:3306/appdb | |
| <%_ } _%> | |
| username: appuser | |
| password: secret |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 2-2: syntax error: mapping values are not allowed here
(syntax)
🤖 Prompt for AI Agents
In generators/server/templates/app/src/main/resources/application-local.yml
around lines 1 to 20, the template emits a top-level spring: only for the
PostgreSQL branch causing invalid YAML for mysql/mariadb and leaves
username/password outside spring.datasource; restructure the template so there
is a single spring: block with one datasource: key (always emitted) and move
datasource.username and datasource.password under it, then inside the datasource
block conditionally set only driver-class-name and url per databaseType
(postgresql/mysql/mariadb) to avoid duplicated datasource keys and incorrect
indentation.
| <%_ if (features.includes('localstack')) { _%> | ||
| ###AWS | ||
| cloud: | ||
| aws: | ||
| region: | ||
| static: us-east-1 | ||
| credentials: | ||
| secret-key: noop | ||
| access-key: noop | ||
| endpoint: http://localhost:4566 | ||
| <%_ } _%> |
There was a problem hiding this comment.
❓ Verification inconclusive
Nest LocalStack config under spring.cloud.aws; verify endpoint keys per version.
- Current block is not under
spring:causing invalid YAML if the DB branch didn’t emitspring:above. - Endpoint scoping may need service‑specific keys.
Minimal safe nesting:
-<%_ if (features.includes('localstack')) { _%>
-###AWS
- cloud:
- aws:
+<%_ if (features.includes('localstack')) { _%>
+ cloud:
+ aws:
region:
static: us-east-1
credentials:
secret-key: noop
access-key: noop
- endpoint: http://localhost:4566
+ endpoint: http://localhost:4566
+ # If using Spring Cloud AWS 3.x+, prefer service-specific endpoints:
+ # sqs:
+ # endpoint: http://localhost:4566
+ # sns:
+ # endpoint: http://localhost:4566
+ # s3:
+ # endpoint: http://localhost:4566
<%_ } _%>Nest LocalStack config under spring.cloud.aws; prefer service-specific endpoints for SC-AWS 3.x.
Move the LocalStack block in generators/server/templates/app/src/main/resources/application-local.yml so its properties are under spring: cloud: aws: — Spring Cloud AWS 3.x migrated cloud.aws.* → spring.cloud.aws.*. (docs.awspring.io)
Prefer service-specific endpoints (e.g. spring.cloud.aws.sqs.endpoint, spring.cloud.aws.sns.endpoint, spring.cloud.aws.s3.endpoint) or the global spring.cloud.aws.endpoint for auto-configured clients; docs show both patterns. (github.com)
If the project intentionally targets SC-AWS 2.x, keep cloud.aws.* or ensure the template emits a top-level spring: block to avoid invalid YAML. (docs.awspring.io)
🤖 Prompt for AI Agents
In generators/server/templates/app/src/main/resources/application-local.yml
around lines 21 to 31, the LocalStack properties are currently under top-level
`cloud.aws` which is invalid for Spring Cloud AWS 3.x; move these properties
under `spring.cloud.aws:` (or emit a top-level `spring:` block if the template
architecture requires it) and prefer service-specific endpoint keys such as
`spring.cloud.aws.sqs.endpoint`, `spring.cloud.aws.sns.endpoint`, and
`spring.cloud.aws.s3.endpoint` (or use `spring.cloud.aws.endpoint` for a global
override) while keeping credentials/region under
`spring.cloud.aws.credentials`/`spring.cloud.aws.region`; if the project
intentionally targets SC-AWS 2.x, preserve `cloud.aws.*` or ensure the YAML
still emits a top-level `spring:` to avoid invalid configuration.
| spring: | ||
| application: | ||
| name: <%= appName %> | ||
| main: | ||
| allow-bean-definition-overriding: 'true' | ||
| jmx: | ||
| enabled: 'false' | ||
| mvc: |
There was a problem hiding this comment.
Duplicate top‑level spring: key; must be a single mapping.
YAML disallows duplicate keys. Merge all spring.* blocks under one spring:.
Apply this consolidation (also fixes quoted booleans/ints):
spring:
application:
name: <%= appName %>
main:
- allow-bean-definition-overriding: 'true'
+ allow-bean-definition-overriding: true
jmx:
- enabled: 'false'
+ enabled: false
mvc:
problemdetails:
- enabled: 'true'
+ enabled: true
-# (second spring: removed; merge sections below into this one)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| spring: | |
| application: | |
| name: <%= appName %> | |
| main: | |
| allow-bean-definition-overriding: 'true' | |
| jmx: | |
| enabled: 'false' | |
| mvc: | |
| spring: | |
| application: | |
| name: <%= appName %> | |
| main: | |
| allow-bean-definition-overriding: true | |
| jmx: | |
| enabled: false | |
| mvc: | |
| problemdetails: | |
| enabled: true |
🤖 Prompt for AI Agents
In generators/server/templates/app/src/main/resources/application.yml around
lines 1 to 8, there are duplicate top-level "spring:" mappings; consolidate all
spring.* entries into a single "spring:" mapping (merge application.name,
main.allow-bean-definition-overriding, jmx.enabled, mvc) under one "spring:" key
with correct indentation, and convert quoted booleans/ints to native YAML
booleans/numbers (e.g., true/false without quotes).
| ################### Database ################### | ||
| spring: | ||
| jpa: | ||
| open-in-view: 'false' | ||
| show-sql: 'false' | ||
| hibernate: | ||
| ddl-auto: none | ||
| properties: | ||
| hibernate: | ||
| jdbc: | ||
| time_zone: UTC | ||
| batch_size: '25' | ||
| lob: | ||
| non_contextual_creation: 'true' | ||
| query: | ||
| fail_on_pagination_over_collection_fetch: 'true' | ||
| plan_cache_max_size: '4096' | ||
| in_clause_parameter_padding: 'true' | ||
| generate_statistics: 'false' | ||
| order_inserts: 'true' | ||
| connection: | ||
| provider_disables_autocommit: 'true' | ||
| order_updates: 'true' | ||
| datasource: | ||
| hikari: | ||
| data-source-properties: | ||
| ApplicationName: ${spring.application.name} | ||
| auto-commit: 'false' | ||
| data: | ||
| jpa: | ||
| repositories: | ||
| bootstrap-mode: deferred | ||
|
|
There was a problem hiding this comment.
Move JPA/datasource under the single spring:; unquote numeric/boolean values.
-################### Database ###################
-spring:
+################### Database ###################
jpa:
- open-in-view: 'false'
- show-sql: 'false'
+ open-in-view: false
+ show-sql: false
hibernate:
ddl-auto: none
properties:
hibernate:
jdbc:
time_zone: UTC
- batch_size: '25'
+ batch_size: 25
lob:
- non_contextual_creation: 'true'
+ non_contextual_creation: true
query:
- fail_on_pagination_over_collection_fetch: 'true'
- plan_cache_max_size: '4096'
- in_clause_parameter_padding: 'true'
- generate_statistics: 'false'
- order_inserts: 'true'
+ fail_on_pagination_over_collection_fetch: true
+ plan_cache_max_size: 4096
+ in_clause_parameter_padding: true
+ generate_statistics: false
+ order_inserts: true
connection:
- provider_disables_autocommit: 'true'
- order_updates: 'true'
+ provider_disables_autocommit: true
+ order_updates: true
datasource:
hikari:
data-source-properties:
ApplicationName: ${spring.application.name}
- auto-commit: 'false'
+ auto-commit: false
data:
jpa:
repositories:
bootstrap-mode: deferred📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ################### Database ################### | |
| spring: | |
| jpa: | |
| open-in-view: 'false' | |
| show-sql: 'false' | |
| hibernate: | |
| ddl-auto: none | |
| properties: | |
| hibernate: | |
| jdbc: | |
| time_zone: UTC | |
| batch_size: '25' | |
| lob: | |
| non_contextual_creation: 'true' | |
| query: | |
| fail_on_pagination_over_collection_fetch: 'true' | |
| plan_cache_max_size: '4096' | |
| in_clause_parameter_padding: 'true' | |
| generate_statistics: 'false' | |
| order_inserts: 'true' | |
| connection: | |
| provider_disables_autocommit: 'true' | |
| order_updates: 'true' | |
| datasource: | |
| hikari: | |
| data-source-properties: | |
| ApplicationName: ${spring.application.name} | |
| auto-commit: 'false' | |
| data: | |
| jpa: | |
| repositories: | |
| bootstrap-mode: deferred | |
| ################### Database ################### | |
| jpa: | |
| open-in-view: false | |
| show-sql: false | |
| hibernate: | |
| ddl-auto: none | |
| properties: | |
| hibernate: | |
| jdbc: | |
| time_zone: UTC | |
| batch_size: 25 | |
| lob: | |
| non_contextual_creation: true | |
| query: | |
| fail_on_pagination_over_collection_fetch: true | |
| plan_cache_max_size: 4096 | |
| in_clause_parameter_padding: true | |
| generate_statistics: false | |
| order_inserts: true | |
| connection: | |
| provider_disables_autocommit: true | |
| order_updates: true | |
| datasource: | |
| hikari: | |
| data-source-properties: | |
| ApplicationName: ${spring.application.name} | |
| auto-commit: false | |
| data: | |
| jpa: | |
| repositories: | |
| bootstrap-mode: deferred |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 26-26: duplication of key "spring" in mapping
(key-duplicates)
🤖 Prompt for AI Agents
generators/server/templates/app/src/main/resources/application.yml around lines
25 to 57: JPA and datasource sections should be nested under the single
top-level spring: key (remove any duplicate spring: blocks) and all
numeric/boolean scalar values must be unquoted; update indentation so spring:
contains jpa:, datasource:, and data: as children, and replace quoted values
like 'false', 'true', '25', '4096' with unquoted booleans/numbers (false, true,
25, 4096) while preserving property names (e.g., ApplicationName:
${spring.application.name}) and existing nested structure.
| <%_ if (dbMigrationTool === 'flywaydb') { _%> | ||
| flyway: | ||
| locations: classpath:/db/migration/{vendor} | ||
| <%_ } _%> | ||
|
|
There was a problem hiding this comment.
Scope Flyway under spring.flyway; fix indentation.
Top‑level flyway won’t bind. Use spring.flyway.
-<%_ if (dbMigrationTool === 'flywaydb') { _%>
- flyway:
- locations: classpath:/db/migration/{vendor}
-<%_ } _%>
+<%_ if (dbMigrationTool === 'flywaydb') { _%>
+ flyway:
+ locations: classpath:/db/migration/{vendor}
+<%_ } _%>(Note: This block is now under the single spring: above.)
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 59-59: syntax error: could not find expected ':'
(syntax)
🤖 Prompt for AI Agents
In generators/server/templates/app/src/main/resources/application.yml around
lines 58 to 62, the Flyway config is declared at top level as "flyway:" which
will not bind; move it under the existing "spring:" key as "spring.flyway:"
(i.e. nest the flyway block under spring) and fix indentation so the "locations:
classpath:/db/migration/{vendor}" line is indented accordingly, preserving the
conditional templating logic.
|
@cchunduri , Can you please address the review comments and write gradle and maven tests with the same. |
3 participants
What is this PR bringing?
As discussed in #182, this PR will bring the option to generate resource config files in YAML format.
Summary by CodeRabbit