Add support for @PostMapping("/path") to pub/sub topic subscriptions by mthmulders · Pull Request #583 · dapr/java-sdk

mthmulders · 2021-07-25T18:58:24Z

Description

I've updated the DaprBeanPostProcessor class so it supports @PostMapping("/path") like annotations. Also, I've added tests for the DaprBeanPostProcessor class.

Issue reference

This PR will close issue #575.

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Extended the documentation

ghost · 2021-07-25T18:58:36Z

All CLA requirements met.

artursouza

Thanks a lot for this pull request. Please, move the integration test to the sdk-tests package - see how there is a way we designed to write those already. So, org.assertj dependency should be removed too.

artursouza · 2021-08-10T00:59:01Z

The integration test code is not in the PR yet. I will give it another look afterwards. Thanks.

mthmulders · 2021-08-10T09:06:48Z

The integration test code is not in the PR yet. I will give it another look afterwards. Thanks.

I've already updated sdk-tests/src/test/java/io/dapr/it/pubsub/http/SubscriberController.java: one out of the many @PostMapping annotated methods there no longer specifies the path argument. All tests that depend on that controller method are still green, which I would consider a proof that the changes have the intended effect.

If you feel that the changes need more tests, could you please elaborate on what kind of test you'd like to see?

codecov · 2021-08-21T02:13:03Z

Codecov Report

Merging #583 (b2a1e22) into master (ac93218) will increase coverage by 0.13%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #583      +/-   ##
============================================
+ Coverage     77.03%   77.16%   +0.13%     
- Complexity      941      942       +1     
============================================
  Files            88       88              
  Lines          3009     3009              
  Branches        334      334              
============================================
+ Hits           2318     2322       +4     
+ Misses          528      525       -3     
+ Partials        163      162       -1

Impacted Files	Coverage Δ
sdk/src/main/java/io/dapr/utils/Retry.java	`75.00% <0.00%> (+25.00%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac93218...b2a1e22. Read the comment docs.

@PostMapping

…apr#583) * Remove duplicate dependency * Transform into managed dependencies * Remove old JUnit version from dependencies * Add tests for DaprBeanPostProcessor * Also register PostMappings that use value * Modify existing pub/sub test to no use @PostMapping(path="...") * Remove added dependencies on AssertJ, Spring Test and Servlet API * Remove test in favour of the sdk-tests one Co-authored-by: Artur Souza <artursouza.ms@outlook.com>

@PostMapping

* Upgrade Mockito to latest version * Run GitHub Actions on Java 11 and 16 * Run JUnit tests with `--add-opens java.base/java.util=ALL-UNNAMED` * Move Surefire JVM arguments to Maven property * Update build.yml * Update validate.yml * simplily build singleton dapr http (#590) Co-authored-by: Artur Souza <artursouza.ms@outlook.com> * Add Automerge workflow (#605) * Add Automerge workflow The automerge workflow will check for complete and approved pull requests and will merge them automatically. Sources: - For the python script to execute the merge: https://raw.githubusercontent.com/dapr/dapr/master/.github/scripts/automerge.py (only changed the label from "automerge" to "auto-merge") - https://raw.githubusercontent.com/dapr/dapr/master/.github/workflows/dapr-bot-schedule.yml (only copied the automerge part, not the prune_stale part as that was not part of the issue. Resolves #603 * Update automerge-bot.yml * Update automerge-bot.yml Co-authored-by: Artur Souza <artursouza.ms@outlook.com> * Cleanup Maven POM files (#604) Added fixed versions for maven-deploy-plugin & spring-boot-maven-plugin as this would otherwise break in a future maven version. Also fixed some indenting and removed duplicate declarations. Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> * Add support for @PostMapping("/path") to pub/sub topic subscriptions (#583) * Remove duplicate dependency * Transform into managed dependencies * Remove old JUnit version from dependencies * Add tests for DaprBeanPostProcessor * Also register PostMappings that use value * Modify existing pub/sub test to no use @PostMapping(path="...") * Remove added dependencies on AssertJ, Spring Test and Servlet API * Remove test in favour of the sdk-tests one Co-authored-by: Artur Souza <artursouza.ms@outlook.com> * Run JUnit tests in all modules with java.base/java.util and java.base/java.lang opened to all modules Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Co-authored-by: 浩 <inf2inf2@outlook.com> Co-authored-by: Tom Cools <tom.cools@live.be> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>

@PostMapping

* Upgrade Mockito to latest version * Run GitHub Actions on Java 11 and 16 * Run JUnit tests with `--add-opens java.base/java.util=ALL-UNNAMED` * Move Surefire JVM arguments to Maven property * Update build.yml * Update validate.yml * simplily build singleton dapr http (dapr#590) Co-authored-by: Artur Souza <artursouza.ms@outlook.com> * Add Automerge workflow (dapr#605) * Add Automerge workflow The automerge workflow will check for complete and approved pull requests and will merge them automatically. Sources: - For the python script to execute the merge: https://raw.githubusercontent.com/dapr/dapr/master/.github/scripts/automerge.py (only changed the label from "automerge" to "auto-merge") - https://raw.githubusercontent.com/dapr/dapr/master/.github/workflows/dapr-bot-schedule.yml (only copied the automerge part, not the prune_stale part as that was not part of the issue. Resolves dapr#603 * Update automerge-bot.yml * Update automerge-bot.yml Co-authored-by: Artur Souza <artursouza.ms@outlook.com> * Cleanup Maven POM files (dapr#604) Added fixed versions for maven-deploy-plugin & spring-boot-maven-plugin as this would otherwise break in a future maven version. Also fixed some indenting and removed duplicate declarations. Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> * Add support for @PostMapping("/path") to pub/sub topic subscriptions (dapr#583) * Remove duplicate dependency * Transform into managed dependencies * Remove old JUnit version from dependencies * Add tests for DaprBeanPostProcessor * Also register PostMappings that use value * Modify existing pub/sub test to no use @PostMapping(path="...") * Remove added dependencies on AssertJ, Spring Test and Servlet API * Remove test in favour of the sdk-tests one Co-authored-by: Artur Souza <artursouza.ms@outlook.com> * Run JUnit tests in all modules with java.base/java.util and java.base/java.lang opened to all modules Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Co-authored-by: 浩 <inf2inf2@outlook.com> Co-authored-by: Tom Cools <tom.cools@live.be> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>

@PostMapping

…apr#583) * Remove duplicate dependency * Transform into managed dependencies * Remove old JUnit version from dependencies * Add tests for DaprBeanPostProcessor * Also register PostMappings that use value * Modify existing pub/sub test to no use @PostMapping(path="...") * Remove added dependencies on AssertJ, Spring Test and Servlet API * Remove test in favour of the sdk-tests one Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>

@PostMapping

* Upgrade Mockito to latest version * Run GitHub Actions on Java 11 and 16 * Run JUnit tests with `--add-opens java.base/java.util=ALL-UNNAMED` * Move Surefire JVM arguments to Maven property * Update build.yml * Update validate.yml * simplily build singleton dapr http (dapr#590) Co-authored-by: Artur Souza <artursouza.ms@outlook.com> * Add Automerge workflow (dapr#605) * Add Automerge workflow The automerge workflow will check for complete and approved pull requests and will merge them automatically. Sources: - For the python script to execute the merge: https://raw.githubusercontent.com/dapr/dapr/master/.github/scripts/automerge.py (only changed the label from "automerge" to "auto-merge") - https://raw.githubusercontent.com/dapr/dapr/master/.github/workflows/dapr-bot-schedule.yml (only copied the automerge part, not the prune_stale part as that was not part of the issue. Resolves dapr#603 * Update automerge-bot.yml * Update automerge-bot.yml Co-authored-by: Artur Souza <artursouza.ms@outlook.com> * Cleanup Maven POM files (dapr#604) Added fixed versions for maven-deploy-plugin & spring-boot-maven-plugin as this would otherwise break in a future maven version. Also fixed some indenting and removed duplicate declarations. Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> * Add support for @PostMapping("/path") to pub/sub topic subscriptions (dapr#583) * Remove duplicate dependency * Transform into managed dependencies * Remove old JUnit version from dependencies * Add tests for DaprBeanPostProcessor * Also register PostMappings that use value * Modify existing pub/sub test to no use @PostMapping(path="...") * Remove added dependencies on AssertJ, Spring Test and Servlet API * Remove test in favour of the sdk-tests one Co-authored-by: Artur Souza <artursouza.ms@outlook.com> * Run JUnit tests in all modules with java.base/java.util and java.base/java.lang opened to all modules Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Co-authored-by: 浩 <inf2inf2@outlook.com> Co-authored-by: Tom Cools <tom.cools@live.be> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>

mthmulders requested review from a team as code owners July 25, 2021 18:58

artursouza suggested changes Jul 28, 2021

View reviewed changes

Comment thread sdk-springboot/src/test/java/io/dapr/springboot/DaprBeanPostProcessorIT.java Outdated

mthmulders added 8 commits July 28, 2021 21:23

Remove duplicate dependency

264b98c

Transform into managed dependencies

b88683a

Remove old JUnit version from dependencies

d7393ed

Add tests for DaprBeanPostProcessor

b45e125

Also register PostMappings that use value

683394e

Modify existing pub/sub test to no use @PostMapping(path="...")

90e213f

Remove added dependencies on AssertJ, Spring Test and Servlet API

75ca4ee

Remove test in favour of the sdk-tests one

1628257

mthmulders force-pushed the pubsub-binding-postmapping-value branch from eb9ac30 to 1628257 Compare July 28, 2021 19:23

Merge branch 'master' into pubsub-binding-postmapping-value

6fb0292

Merge branch 'master' into pubsub-binding-postmapping-value

8be5ae4

artursouza previously approved these changes Aug 14, 2021

View reviewed changes

artursouza self-assigned this Aug 14, 2021

artursouza added 3 commits August 17, 2021 23:56

Merge branch 'master' into pubsub-binding-postmapping-value

12b2b6d

Merge branch 'master' into pubsub-binding-postmapping-value

4f959e6

Merge branch 'master' into pubsub-binding-postmapping-value

f4f660e

artursouza added the auto-merge label Aug 21, 2021

Merge branch 'master' into pubsub-binding-postmapping-value

d19ba10

artursouza dismissed their stale review via b2a1e22 August 21, 2021 02:09

Merge branch 'master' into pubsub-binding-postmapping-value

b2a1e22

artursouza approved these changes Aug 21, 2021

View reviewed changes

dapr-bot merged commit 3c3a527 into dapr:master Aug 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for @PostMapping("/path") to pub/sub topic subscriptions#583

Add support for @PostMapping("/path") to pub/sub topic subscriptions#583
dapr-bot merged 15 commits into
dapr:masterfrom
mthmulders:pubsub-binding-postmapping-value

mthmulders commented Jul 25, 2021

Uh oh!

ghost commented Jul 25, 2021 •

edited by ghost

Loading

Uh oh!

artursouza left a comment

Uh oh!

Uh oh!

artursouza commented Aug 10, 2021 •

edited

Loading

Uh oh!

mthmulders commented Aug 10, 2021

Uh oh!

codecov Bot commented Aug 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mthmulders commented Jul 25, 2021

Description

Issue reference

Checklist

Uh oh!

ghost commented Jul 25, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

artursouza left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

artursouza commented Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mthmulders commented Aug 10, 2021

Uh oh!

codecov Bot commented Aug 21, 2021

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ghost commented Jul 25, 2021 •

edited by ghost

Loading

artursouza commented Aug 10, 2021 •

edited

Loading