-
Notifications
You must be signed in to change notification settings - Fork 54
Feature/onion request refactor #1792
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
base: dev
Are you sure you want to change the base?
Conversation
| ) | ||
|
|
||
| val json = runCatching { | ||
| JsonUtil.fromJson(responseBytes, Map::class.java) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it makes sense to migrate to use kotlinx serialization here, since we are rebuilding this now
| fun getForkInfo(): ForkInfo | ||
| fun setForkInfo(forkInfo: ForkInfo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are these used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for some work that was performed in the OnionRequest handleResponse before:
if (body.containsKey("hf")) {
@Suppress("UNCHECKED_CAST")
val currentHf = body["hf"] as List<Int>
if (currentHf.size < 2) {
Log.e("Loki", "Response contains fork information but doesn't have a hard and soft number")
} else {
val hf = currentHf[0]
val sf = currentHf[1]
val newForkInfo = ForkInfo(hf, sf)
if (newForkInfo > SnodeAPI.forkInfo) {
SnodeAPI.forkInfo = ForkInfo(hf,sf)
} else if (newForkInfo < SnodeAPI.forkInfo) {
Log.w("Loki", "Got a new snode info fork version that was $newForkInfo, less than current known ${SnodeAPI.forkInfo}")
}
}
}
```
Moved it to a different place for separation of concerns
| if (body == null || body.isEmpty()) return false | ||
|
|
||
| val json: Map<*, *> = try { | ||
| JsonUtil.fromJson(body, Map::class.java) as Map<*, *> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same re: json serialization
app/src/main/java/org/session/libsession/messaging/jobs/NotifyPNServerJob.kt
Show resolved
Hide resolved
| * | ||
| */ | ||
| suspend fun send( | ||
| path: List<Snode>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there's a Path type defined, should we use it here too?
WIP
Early stages of the first networking refactor