Skip to content
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

Feature/1204/247 api client [Spike] #268

Closed
wants to merge 14 commits into from

Conversation

algattik
Copy link

@algattik algattik commented May 16, 2022

What this PR changes/adds

Add generated REST client module, and use it system tests.
Fix serialised classes that were not compatible with Swagger OpenAPI generator.

Why it does that

  • Reduce the verbosity, and improve the clarity of system tests
  • Ensure the scope of system tests is not integration testing of the controllers, that can be done in specific RestAssured tests
  • Provide a client module in Maven that can be directly used by end users running on JVM
  • Dogfood our OpenAPI definition (value shown by discrepancies discovered and fixed in this PR)

Further notes

Several classes needed to be updated as they used Jackson patterns that are not compatible with Swagger generator. That is, the OpenAPI definition was previously broken.

  • Permission and Duty: stop using JsonTypeName. This type of JSON inheritance is only useful when the superclass is used in API. Here only subclasses are used in API, and Swagger generator does not handle this case. See Generating definition for polymorphic types when only subclass is used swagger-api/swagger-core#4181
  • PolicyType stop using custom deserialisation code. This is not compatible with Swagger generator
  • DataPlaneInstance and DataPlaneInstanceImpl: Added annotations so that Swagger generator detects JSON inheritance scenario.

Linked Issue(s)

#247

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • added relevant details to the changelog? (skip with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and styleguide for details)

@algattik algattik changed the base branch from main to feature/1204-api-client May 16, 2022 04:45
@github-actions
Copy link

github-actions bot commented May 16, 2022

Unit Test Results

     399 files  ±0       399 suites  ±0   11m 13s ⏱️ -53s
12 164 tests ±0  12 140 ✔️  - 1  18 💤 ±0  6 +1 
12 223 runs  ±0  12 199 ✔️  - 1  18 💤 ±0  6 +1 

For more details on these failures, see this check.

Results for commit 3019512. ± Comparison against base commit 70271b5.

♻️ This comment has been updated with latest results.

@codecov-commenter
Copy link

codecov-commenter commented May 16, 2022

Codecov Report

Merging #268 (3019512) into feature/1204-api-client (70271b5) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@                     Coverage Diff                     @@
##           feature/1204-api-client     #268      +/-   ##
===========================================================
+ Coverage                    67.51%   67.53%   +0.01%     
===========================================================
  Files                          717      717              
  Lines                        15749    15738      -11     
  Branches                      1051     1049       -2     
===========================================================
- Hits                         10633    10628       -5     
+ Misses                        4633     4628       -5     
+ Partials                       483      482       -1     
Impacted Files Coverage Δ
.../eclipse/dataspaceconnector/policy/model/Duty.java 92.85% <ø> (ø)
...se/dataspaceconnector/policy/model/Permission.java 93.10% <ø> (ø)
...clipse/dataspaceconnector/policy/model/Policy.java 81.33% <ø> (ø)
.../eclipse/dataspaceconnector/policy/model/Rule.java 100.00% <ø> (ø)
...plane/selector/instance/DataPlaneInstanceImpl.java 90.00% <ø> (ø)
...se/dataspaceconnector/policy/model/PolicyType.java 100.00% <100.00%> (+46.15%) ⬆️

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 70271b5...3019512. Read the comment docs.

@algattik algattik linked an issue May 16, 2022 that may be closed by this pull request
5 tasks
@algattik algattik changed the title Feature/1204/247 api client Feature/1204/247 api client [Spike] Jun 8, 2022
@algattik algattik closed this Jun 8, 2022
@algattik algattik added the spike label Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spike: Data Management API Client
2 participants