#19640: review and improve interface hierarchy ---------------------------------+----------------------------------- Reporter: iwakeh | Owner: metrics-team Type: enhancement | Status: assigned Priority: Medium | Milestone: metrics-lib 2.0.0 Component: Metrics/metrics-lib | Version: Severity: Normal | Resolution: Keywords: | Actual Points: Parent ID: | Points: Reviewer: | Sponsor: ---------------------------------+----------------------------------- Changes (by karsten):
* owner: karsten => metrics-team * status: new => assigned Comment: I'd like us to flesh out how we'd like to improve the interface hierarchy, and I'll start by going through the ideas above. 1. Comment 10 of #19398 says: "There shouldn't be two interfaces declaring the very same method; there should be a more structured hierarchy for the interfaces, too." I think I agree in theory, though I'd want to discuss how strictly we want to implement this principle. Here's a contrasting principle: let's avoid introducing interfaces that have the sole purpose of deduplicating methods. I think we need to find a middle ground here. 2. The linked comment in `NetworkStatusImpl` seems related to possible improvements to the interface hierarchy, though I'd prefer if we address that issue after improving the interface hierarchy. 3. Comment 4 above, splitting `ParseHelper` into interface and implementation seems unrelated, unless we're planning to make the interface part of the metrics-lib API. Right now it's solely an implementation helper to avoid duplicating code. 4. #17861 is indeed something where we could improve the interface hierarchy, by having a separate interface for each descriptor type. But I'm not sure how to best implement such a new interface. See the heavy overlap between `RelayNetworkStatusVote` and `RelayNetworkStatusConsensus`. A new `RelayNetworkStatusMicrodescConsensus` interface following the current code practice would basically copy over the entire consensus interface and change a method here and there. We should avoid that. In addition to those items, I came up with a few more ideas: 5. When we added `RelayServerDescriptor`, `RelayExtraInfoDescriptor`, `BridgeServerDescriptor`, and `BridgeExtraInfoDescriptor`, we left all methods in the superinterfaces `ServerDescriptor` and `ExtraInfoDescriptor`. We should consider moving methods that are specific to relays or bridges to the subinterfaces. 6. We're still using a single `NetworkStatusEntry` for entries in all the different network statuses. We should consider using an interface hierarchy there, too, to only provide relevant methods depending on the network status at hand. I started working on a patch, but I'd like to keep this discussion on the conceptual level for now. And here are some more concrete suggestions for improving the interface hierarchy, somewhat overlapping with the ideas above: 7. Introduce a `NetworkStatus` interface to capture all common parts of network statuses in directory protocol versions 2 and 3, including all `RelayNetworkStatus*` and `BridgeNetworkStatus`. 8. Introduce another interface called `NetworkStatusVote` for common parts of network statuses in directory protocol version 3, following the common part in URLs like `http://<hostname>/tor/status- vote/next/authority.z`. Though there's the risk that this interface will be confused with `RelayNetworkStatusVote`. Another possible interface name could be `RelayNetworkStatusVersion3`, though I'm not a big fan of including numbers in type names. 9. Introduce a new interface `NetworkStatus.Entry` just like `ExitList.Entry` and a set of subinterfaces like `RelayNetworkStatusConsensus.Entry` to address issue 6 above. Requires some heavy generics lifting. 10. Use `RelayNetworkStatusConsensus` for the common parts in consensuses of any flavor and introduce `RelayNetworkStatusNsConsensus` for unflavored /ns-flavored and `RelayNetworkStatusMicrodescConsensus` for microdesc- flavored consensuses. That's basically what we did with adding new subinterfaces to `ServerDescriptor`. What else can/should we do? Or which parts should we rather not do? -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19640#comment:7> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online _______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs