Refactor remote connection session establishment logic #1210
Refactor remote connection session establishment logic #1210Funkatronics merged 15 commits intomainfrom
Conversation
| override fun onScenarioComplete() = Unit | ||
| override fun onScenarioError() = Unit | ||
| override fun onScenarioTeardownComplete() { | ||
| if (sessionId == null) return |
There was a problem hiding this comment.
Is it safe to skip emitting SessionTerminated?
There was a problem hiding this comment.
it is, as long as the wallet knows when to close itself - this is fakewallet keep in mind. This logic isnt very clean tho - I'll see if I can improve it.
| <string name="label_failed_session_establishment">Connection Failed</string> | ||
| <string name="str_failed_session_establishment"> | ||
| Could not establish a session with the dapp, the connection failed. | ||
| Check your network connection |
There was a problem hiding this comment.
"Check your network connection and try again"?
| protected String mActiveSessionId = null; | ||
| @Nullable | ||
| @GuardedBy("mLock") | ||
| private NotifyingCompletableFuture<String> mSessionEstablishedFuture; |
There was a problem hiding this comment.
Initialize with null like other nullable members?
|
|
||
| @Override | ||
| @Nullable | ||
| @GuardedBy("mLock") |
There was a problem hiding this comment.
no mLock is private
There was a problem hiding this comment.
oh wait I see what you are saying, this is the getter. probably shouldnt be listed as guarded
There was a problem hiding this comment.
I decided to remove the getter entirely. I realized there were many states where the session id would be null and it would be confusing to the consumer. We can add getters in the future if there is a need.
…sure its valid in the right states
2 participants
Breaking change:
Scenario.starthas been deprecated and repalced withScenario.startAsync. Any classes implementing theScenariointerface must implement this new method.