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

feat: adopt v7 API #WPB-11975 #3099

Merged
merged 10 commits into from
Nov 18, 2024
Merged

feat: adopt v7 API #WPB-11975 #3099

merged 10 commits into from
Nov 18, 2024

Conversation

damian-kaczmarek
Copy link
Contributor

@damian-kaczmarek damian-kaczmarek commented Nov 12, 2024

TaskWPB-11975 [Android] Adopt API v7


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

New API version added on backend https://wearezeta.atlassian.net/wiki/spaces/ENGINEERIN/pages/1309868033/API+changes+v6+v7

Important

Please verify this list, as part of the CR.

Changes listed in doc (applied ✅ , not relevant ❌ )

  • The capabilities field of the Client structure now contains a list of capabilities, rather than a nested object. In the notifications containing the Client structure, it will still be a nested object. ✅

  • The notification contains the old format {capabilities: { capabilities: [...] }}, but clients must allow both old and new format starting from V6. Once V5 is discontinued, the backend will start sending notifications in the new form.✅

  • There is a new type of capability in capabilities field for Client: consumable-notifications. This will only show up when using API v7. In the notifications containing the Client structure, this capability will not be visible. ❌

  • DELETE /oauth/applications/:id/ is removed in v7 in favor of a new endpoint DELETE /oauth/applications/:id/sessions which takes a request body with an optional password. The password is verified in case of the user is not an SSO user.❌

  • renamed POST /users/handles → POST /handles❌

  • renamed POST /users/handles → POST /handles❌

  • renamed HEAD /users/handles → HEAD /handles❌

  • renamed POST /conversations/bots → POST /bot/conversations❌

  • renamed DELETE /conversations/bots → DELETE /bot/conversations❌

  • renamed POST /conversations/one2one → POST /one2one-conversations ✅

  • renamed GET /conversations/one2one → GET /one2one-conversations ✅

  • GET /scim/auth-tokens returns an array of ScimTokenInfoitems which now have an additional field: name.❌

  • POST /scim/auth-tokens response has a new optional field: name. If not specified, the name will be set to the ID of the SCIM token by the backend.❌

  • GET /teams/invitations/info response has a new optional field created_by_email which will be occupied with the inviter’s email in case the invitee is a personal user.❌

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

This is more of a general play test if everything works.
There were no functional changes.

To test you need to switch to anta
https://wearezeta.atlassian.net/wiki/spaces/ENGINEERIN/pages/300351887/Testing+environments+for+federation#anta

The html with deeplinks switching can be downloaded here. You must logout first before switching. https://wearezeta.atlassian.net/wiki/spaces/ENGINEERIN/pages/300351887/Testing+environments+for+federation#anta

To get signup code you must read the email from inbucket.
Password is on 1password.


PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

Copy link
Member

@MohamadJaara MohamadJaara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion to remove not used modifiers from generated api classes and missing test for the CapabilitiesDeserializer

Copy link
Contributor

github-actions bot commented Nov 12, 2024

Test Results

3 275 tests  +17   3 168 ✅ +17   5m 3s ⏱️ +54s
  559 suites + 2     107 💤 ± 0 
  559 files   + 2       0 ❌ ± 0 

Results for commit 2c383ac. ± Comparison against base commit f417fd4.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Nov 12, 2024

🐰 Bencher Report

Branchfeature/api-v7
Testbedubuntu-latest

⚠️ WARNING: The following Measure does not have a Threshold. Without a Threshold, no Alerts will ever be generated!

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results
BenchmarkLatencynanoseconds (ns)
com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInFiles📈 view plot
⚠️ NO THRESHOLD
660,799.41
com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInMemory📈 view plot
⚠️ NO THRESHOLD
419,816,295.40
com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.messageInsertionBenchmark📈 view plot
⚠️ NO THRESHOLD
1,330,214,405.17
com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.queryMessagesBenchmark📈 view plot
⚠️ NO THRESHOLD
22,073,418.55
🐰 View full continuous benchmarking report in Bencher

@datadog-wireapp
Copy link

datadog-wireapp bot commented Nov 12, 2024

Datadog Report

Branch report: feature/api-v7
Commit report: 5de294d
Test service: kalium-jvm

✅ 0 Failed, 3168 Passed, 107 Skipped, 39.08s Total Time

@damian-kaczmarek damian-kaczmarek changed the title feature: adopt v7 API #WPB-11975 - adding tests rests can be checked feature: adopt v7 API #WPB-11975 Nov 13, 2024
@damian-kaczmarek damian-kaczmarek changed the title feature: adopt v7 API #WPB-11975 feat: adopt v7 API #WPB-11975 Nov 13, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 9.06040% with 271 lines in your changes missing coverage. Please review.

Project coverage is 54.01%. Comparing base (f417fd4) to head (2c383ac).
Report is 6 commits behind head on develop.

Files with missing lines Patch % Lines
...responses/conversation/ConversationResponseJson.kt 0.00% 82 Missing ⚠️
...etworkContainer/AuthenticatedNetworkContainerV7.kt 0.00% 45 Missing ⚠️
.../wire/kalium/mocks/responses/ClientResponseJson.kt 0.00% 28 Missing ⚠️
...workContainer/UnauthenticatedNetworkContainerV7.kt 0.00% 28 Missing ⚠️
...com/wire/kalium/network/KaliumKtorCustomLogging.kt 0.00% 9 Missing ⚠️
.../networkContainer/AuthenticatedNetworkContainer.kt 0.00% 7 Missing ⚠️
...i/authenticated/client/CapabilitiesDeserializer.kt 0.00% 6 Missing ⚠️
...etworkContainer/UnauthenticatedNetworkContainer.kt 0.00% 6 Missing ⚠️
...n/kotlin/com/wire/kalium/logic/data/event/Event.kt 0.00% 2 Missing ⚠️
...alium/mocks/responses/ListOfClientsResponseJson.kt 0.00% 2 Missing ⚠️
... and 30 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3099      +/-   ##
===========================================
+ Coverage    52.60%   54.01%   +1.40%     
===========================================
  Files         1323     1223     -100     
  Lines        51595    35769   -15826     
  Branches      4783     3630    -1153     
===========================================
- Hits         27141    19320    -7821     
+ Misses       22492    15049    -7443     
+ Partials      1962     1400     -562     
Files with missing lines Coverage Δ
...ceiver/conversation/NewConversationEventHandler.kt 92.72% <100.00%> (+1.81%) ⬆️
...lin/com/wire/kalium/network/BackendMetaDataUtil.kt 100.00% <100.00%> (ø)
...kotlin/com/wire/kalium/network/KaliumHttpLogger.kt 0.00% <ø> (ø)
...base/authenticated/conversation/ConversationApi.kt 40.00% <ø> (ø)
.../network/api/v0/authenticated/ConversationApiV0.kt 60.33% <ø> (+1.95%) ⬆️
...kalium/network/api/v2/authenticated/PreKeyApiV2.kt 100.00% <ø> (ø)
...um/network/api/v2/authenticated/PropertiesApiV2.kt 0.00% <ø> (ø)
.../network/api/v3/authenticated/ConversationApiV3.kt 89.74% <ø> (+15.27%) ⬆️
...um/network/api/v3/authenticated/PropertiesApiV3.kt 0.00% <ø> (ø)
...m/network/api/v4/authenticated/AccessTokenApiV4.kt 0.00% <ø> (ø)
... and 50 more

... and 136 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

@damian-kaczmarek damian-kaczmarek marked this pull request as ready for review November 14, 2024 12:44
@ohassine ohassine added this pull request to the merge queue Nov 18, 2024
Merged via the queue into develop with commit 94d387e Nov 18, 2024
22 checks passed
@ohassine ohassine deleted the feature/api-v7 branch November 18, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants