Add Grails 8 email guide sample app. - #1
Conversation
jamesfredley
left a comment
jamesfredley
left a comment
There was a problem hiding this comment.
Thank you, @sanjana2505006, for the Grails 8 sample-app port. The blockers center on the three EmailService implementations behaving inconsistently on failure, allowing unsuccessful provider sends to appear successful.
Additional requested changes without eligible inline anchors:
- complete/src/main/groovy/example/AwsSesMailService.groovy: rename AwsSesMailService to AwsSesEmailService to match the EmailService boundary naming.
- complete/grails-app/controllers/example/MailController.groovy: return validation details instead of a bare 422, for example
respond cmd.errors, view: '/application/errors', status: UNPROCESSABLE_ENTITY.
Body-only repository items:
- Delete the duplicate, unused Gradle wrapper directories at complete/wrapper/ and initial/wrapper/. The application wrappers use gradle/wrapper/.
- Restore recursive Gradle/build exclusions in the root .gitignore and restore IDE and OS exclusions such as .idea, *.iml, and .DS_Store in nested app .gitignore files.
Please also coordinate these behavior and documentation updates with companion website PR apache/grails-static-website#524.
| ) | ||
|
|
||
| then: | ||
| resp.statusCode == HttpStatus.OK |
There was a problem hiding this comment.
This spec covers only the happy path. Add invalid-request cases, such as a missing recipient or subject, that assert 422 and the JSON validation-error schema. This protects the API contract and the errors view.
| Errors errorsObject = (Errors)this.errors | ||
| def allErrors = errorsObject.allErrors | ||
| int errorCount = allErrors.size() | ||
| def resourcePath = g.link(resource:request.uri, absolute:false) |
There was a problem hiding this comment.
The live invalid-request response for POST /mail/send currently reports path: /send and a self link ending in /send. g.link(resource: request.uri) treats the URI as a resource name. Please preserve the actual request URI and use g.link(uri: ..., absolute: true) for the self link, then assert the exact path and href in both invalid-request tests.
| respond cmd.errors, view: '/application/errors', status: UNPROCESSABLE_ENTITY | ||
| return | ||
| } | ||
| log.info '{}', cmd.toString() |
There was a problem hiding this comment.
EmailCmd has @ToString, so this INFO log writes recipient, cc, bcc, subject, and full HTML or text bodies. The integration run showed the message content in application logs. Please remove the full command logging or log only non-sensitive metadata.
|
Latest-head re-review found one additional correctness blocker that GitHub could not anchor at its unchanged helper lines:
I also added inline comments for the reproduced integration-test failure, the incorrect validation path/self link, and full email-content logging. The original addressed threads were resolved; the invalid-request-test thread remains open because the prescribed suite is still red. |
|
@jamesfredley addressed the feedback |
| Errors errorsObject = (Errors)this.errors | ||
| def allErrors = errorsObject.allErrors | ||
| int errorCount = allErrors.size() | ||
| def resourcePath = request.uri |
There was a problem hiding this comment.
Reproduced on current head b07505d: ./gradlew test integrationTest still fails both invalid-request cases, and the GitHub complete job is red. request.uri remains /send after the error-view dispatch, so the JSON path and self link still omit /mail. Please derive both values from the original servlet request URI, such as request.requestURI, verify the live payload is /mail/send, and rerun the full command.
| return | ||
| } | ||
| log.info '{}', cmd.toString() | ||
| log.info 'Sending mail to {} with subject {}', cmd.recipient, cmd.subject |
There was a problem hiding this comment.
This is better than logging the full command, but recipient addresses are personal data and subjects can contain sensitive content, so they are not non-sensitive metadata. Please log only a generic send event, correlation ID, or provider message ID at INFO.
| implementation "org.springframework.boot:spring-boot-tomcat" | ||
| console "org.apache.grails:grails-console" | ||
| //tag::mailPlugin[] | ||
| implementation "org.grails.plugins:grails-mail:$mailPluginVersion" |
There was a problem hiding this comment.
Fresh ./gradlew bootRun does not start the application. The context fails because grails-mail requires a org.grails.gsp.GroovyPagesTemplateEngine bean, but this REST app has no Grails GSP dependency; Gradle misleadingly exits successful after APPLICATION FAILED TO START. Please add the required org.apache.grails:grails-gsp runtime support (and GSP Gradle plugin if templates will be compiled), verify port 8080 actually binds, and add a startup smoke check so CI catches this surface.
2 participants
Summary
Ports the grails-email sample app to Grails 8 (
grails8branch).initial/as a Grails 8 REST API starter (JDK 21, Apache Grails BOM)complete/with:EmailServiceboundary + SMTP (grails-mail5.x), SendGrid, and AWS SES implementationsapplication.yml(email.provider, SMTP / SendGrid / SES settings)resources.groovy(skipped intestso integration tests can mock the sender)POST /mail/sendcontroller andEmailCmdcommand objectEmailServiceand asserts message fields — no live email in CI.github/workflows/grails8.ymland updates READMECompanion website PR (prose +
guides.ymlv8 block):apache/grails-static-website