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

Refactor FederationServer #234

Open
jillesvangurp opened this issue Jul 19, 2019 · 0 comments
Open

Refactor FederationServer #234

jillesvangurp opened this issue Jul 19, 2019 · 0 comments

Comments

@jillesvangurp
Copy link
Contributor

The current FederationServer is a bit weird in the sense that you create an instance every time you want to validate a single address.

I propose to make this a bit less convoluted as follows:

  • FederationAddressResolver that has a resolve method that returns the response and a constructor injected okhttpclient. There should be no need to have separate resolvers for each domain.
  • separate the address parsing into a separate static method so that we can test this without needing a mock http server.
  • Likewise mocking a remote server to verify that we can deserialize a json response is not something that needs unit testing. At best we need a simple test that verifies that given a valid response, the deserialization works.
  • get rid of the static https variable and the static method for creating an okhttp instance; doing this per address is a bad idea. Global variables should not exist in production code.

I think it's safe to do this as obviously not a lot of people are aware this even exists or they would have complained ;-). Happy to do the work if people like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants
@jillesvangurp and others