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.

Sign up for GitHub

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

Jump to bottom

feat: updates session manager sdk and related changes. #96

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

feat: updates session manager sdk and related changes. #96

wants to merge 36 commits into from

Conversation

grvgoel81
Copy link
Contributor

@grvgoel81 grvgoel81 commented Sep 3, 2024 •
edited
Loading

Motivation and Context

  • Update session-manager-android sdk
  • Update authService version to v9 and walletServices to v3.

Jira Link:
https://toruslabs.atlassian.net/browse/PD-4111

Description

  • Update session-manager-android sdk and related changes wrt API method parameters.
  • Update minSdkVersion to 26 in build.gradle
  • Handling Concurrency and update example.
  • Update authService version to v9 and walletServices to v3.
  • Update request() method to return signResponse.

How has this been tested?

  • Tested all functionalities like login, logout, session management ,walletservices, setupMFA and request() method.

Screenshots (if appropriate):

Screen.Recording.2024-11-05.at.11.12.10.AM.mov

For request method changes:

Screen.Recording.2024-10-23.at.10.00.39.AM.mov

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project. (run lint)
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@grvgoel81
feat: updates session manager sdk and related changes.
8554ad2 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
metalurgical
metalurgical requested changes Sep 3, 2024
View reviewed changes
Copy link

@metalurgical metalurgical left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Have added some comments.

feat: PR comments addressed
3716a1e 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
metalurgical and others added 6 commits September 3, 2024 08:29
@metalurgical
cleanup
ac750f4
@metalurgical
remove network checking where underlying library already does this, a…
f0dad11 
…dd todos
@metalurgical
additional todos
9890858
@grvgoel81
feat: Resolved TODO's
bebb70b 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: Resolved more TODO's
8eda6d0 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: update session-manager-android sdk version
1dc8474 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@metalurgical
Copy link

metalurgical commented Sep 3, 2024 •
edited
Loading

@grvgoel81 Looking a lot better, please look at the rest of the todos

@grvgoel81
feat: update session-manager-android sdk version
b005d68 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@metalurgical metalurgical dismissed their stale review September 10, 2024 07:26

comments resolved

grvgoel81 and others added 9 commits September 10, 2024 13:37
@grvgoel81
feat: resolved PR comments
96f0717 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
feat: update session-manager-sdk version
fdfb714 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: update authServiceVersion to v9 and walletServicesVersion to v3
4d30233 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: update session manager changes
5877ad5 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: update session manager changes
a482671 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: remove runOnUiThread from example and code cleanup
705f718 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: code cleanup
1d3b3ac 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: code cleanup
2db034e 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: Update session-manager-sdk
e6a1f22 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: Update session-manager-sdk
6be5463 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: code refactored
57e779c 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: code refactored
1421cc5 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: code refactored
d48e4a9 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
Gaurav Goel and others added 9 commits October 9, 2024 08:30
Merge branch 'master' into feat/update-session-manager-changes
22df356 
# Conflicts:
#	core/src/main/java/com/web3auth/core/Web3Auth.kt
@grvgoel81
feat: Resolve conflicts
c07c2c7 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: code refactored
e73750a 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: Update fetchProjectConfig API inout params
2adec20 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
feat: Add appState in SignMessage.kt
695a354 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
feat: code refactored
2b474a9 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: code refactored for request method an update example
91dd944 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: add webview cancelled event.
af4c2d2 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: update request function return type from SignResponse to generi…
bc36812 
…c JsonObject

Signed-off-by: Gaurav Goel <gaurav@tor.us>
grvgoel81 and others added 4 commits October 29, 2024 07:26
@grvgoel81
feat: code refactored
cf5e125 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: updated code for session-manager changes
6e33a62 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
@grvgoel81
feat: updated code for session-manager changes
6c3e8c5 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
feat: updated session-manager dep
ee1bb79 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
hartmannj
hartmannj reviewed Nov 7, 2024
View reviewed changes
core/src/main/java/com/web3auth/core/Web3Auth.kt Outdated
} else {
initializeCf.completeExceptionally(error)
}
initializeCf.complete(null)
Copy link

Choose a reason for hiding this comment

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

Is it fine to call both
initializeCf.completeExceptionally(error)
and
initializeCf.complete(null)
right after each other in case of error?

Copy link
Contributor Author

@grvgoel81 grvgoel81 Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

But what's the need of it? Since .completeExceptionally completes the future with exception and .complete completes the future with any result. So ideally either of them should be called.

Copy link

Choose a reason for hiding this comment

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

Should not make a difference.

According to the docs it is conditional on it not already being completed for both functions:
https://docs.oracle.com/javase/8/docs/api/index.html?java/util/concurrent/CompletableFuture.html

Copy link

Choose a reason for hiding this comment

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

@grvgoel81 For clarity, it may be better to move line 178 to just after line 174 here.

@grvgoel81
code refactor
225eed9 
Signed-off-by: Gaurav Goel <gaurav@tor.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@metalurgical metalurgical metalurgical left review comments

@hartmannj hartmannj hartmannj left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@grvgoel81 @metalurgical @hartmannj