Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

HTTP Proxy: Separate Saving Jump Service Addresses from Message Processing #845

Closed
coneiric opened this issue Mar 29, 2018 · 2 comments
Closed

Comments

@coneiric
Copy link
Contributor

coneiric commented Mar 29, 2018


By submitting this issue, I confirm the following:

  • I have read and understood the contributor guide in kovri-docs.
  • I have checked that the issue I am reporting can be replicated or that the feature I am suggesting is not present.
  • I have checked opened or recently closed pull requests for existing solutions/implementations to my issue/suggestion.

After IRC collaboration with @anonimal & @brbzull0, it seems to me that HTTPMessage::SaveJumpServiceAddress should really be its own async proxy handler, HTTPProxyHandler::SaveAddress.

Refactoring saving addresses as a proxy handler will decouple HTTP message processing from AddressBook's timer + mutex (allowing us to get rid of the hack needed in #838).

References #340 & #844.

@coneiric coneiric changed the title HTTPMessage: Separate Saving Jump Service Addresses from Message Processing HTTP Proxy: Separate Saving Jump Service Addresses from Message Processing Mar 29, 2018
@coneiric
Copy link
Contributor Author

coneiric commented Mar 30, 2018

Would it be better design to have the handler send the address over the socket as a NetDB DatabaseStoreMessage, or directly insert to the AddressBook?

edit: NetDB is a different submodule, should insert to AddressBook from an async proxy handler, after verifying the address is reachable. Thanks for clarification over IRC @anonimal 👍

Thought you might have been hinting at the former on IRC @anonimal (was mistaken, see above):

@anonimal Right off the bat, IMHO that should be at least 4 different classes. The socket/handler could hypothetically be templated, not specific to HTTP but we'll ignore that for now.
@anonimal From there you can almost certainly decouple the work even further.
@anonimal In that scheme, I don't see a reliance on inheritance but really there are multiple ways to deal with the issue.
@anonimal Did that make sense?
@anonimal Notice how I left out addressbook and storage, etc. This is intentional.

@anonimal
Copy link
Collaborator

anonimal commented Sep 7, 2018

NOTICE: THIS ISSUE HAS BEEN MOVED TO GitLab. Please continue the discussion there. See #1013 for details.

@anonimal anonimal closed this as completed Sep 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants