cleanup: netx does not use netxlite legacy names

This diff refactors netx and netxlite to ensure we're not using netxlite legacy names inside of netx. To this end, we're cheating a bit. We're exposing a new factory to get an unwrapped stdlib resolver rather than defining a legacy name to export the private name of the same factory. This is actually a fine place to stop, for now, the next and netxlite refactoring at ooni/probe#2121.
ooni · Jun 6, 2022 · 48476cc · 48476cc
1 parent 64bffbd
commit 48476cc
Show file tree

Hide file tree

Showing 18 changed files with 33 additions and 42 deletions.
diff --git a/internal/engine/experiment/ndt7/dial.go b/internal/engine/experiment/ndt7/dial.go
@@ -30,7 +30,7 @@ func newDialManager(ndt7URL string, logger model.Logger, userAgent string) dialM
 }
 
 func (mgr dialManager) dialWithTestName(ctx context.Context, testName string) (*websocket.Conn, error) {
-	reso := netxlite.NewResolverStdlib(mgr.logger)
+	reso := netxlite.NewStdlibResolver(mgr.logger)
 	dlr := netxlite.NewDialerWithResolver(mgr.logger, reso)
 	dlr = bytecounter.WrapWithContextAwareDialer(dlr)
 	// Implements shaping if the user builds using `-tags shaping`

diff --git a/internal/engine/geolocate/geolocate.go b/internal/engine/geolocate/geolocate.go
@@ -114,7 +114,7 @@ func NewTask(config Config) *Task {
 		config.UserAgent = fmt.Sprintf("ooniprobe-engine/%s", version.Version)
 	}
 	if config.Resolver == nil {
-		config.Resolver = netxlite.NewResolverStdlib(config.Logger)
+		config.Resolver = netxlite.NewStdlibResolver(config.Logger)
 	}
 	return &Task{
 		countryLookupper:     mmdbLookupper{},

diff --git a/internal/engine/geolocate/iplookup_test.go b/internal/engine/geolocate/iplookup_test.go
@@ -14,7 +14,7 @@ import (
 func TestIPLookupGood(t *testing.T) {
 	ip, err := (ipLookupClient{
 		Logger:    log.Log,
-		Resolver:  netxlite.NewResolverStdlib(model.DiscardLogger),
+		Resolver:  netxlite.NewStdlibResolver(model.DiscardLogger),
 		UserAgent: "ooniprobe-engine/0.1.0",
 	}).LookupProbeIP(context.Background())
 	if err != nil {
@@ -30,7 +30,7 @@ func TestIPLookupAllFailed(t *testing.T) {
 	cancel() // immediately cancel to cause Do() to fail
 	ip, err := (ipLookupClient{
 		Logger:    log.Log,
-		Resolver:  netxlite.NewResolverStdlib(model.DiscardLogger),
+		Resolver:  netxlite.NewStdlibResolver(model.DiscardLogger),
 		UserAgent: "ooniprobe-engine/0.1.0",
 	}).LookupProbeIP(ctx)
 	if !errors.Is(err, context.Canceled) {
@@ -45,7 +45,7 @@ func TestIPLookupInvalidIP(t *testing.T) {
 	ctx := context.Background()
 	ip, err := (ipLookupClient{
 		Logger:    log.Log,
-		Resolver:  netxlite.NewResolverStdlib(model.DiscardLogger),
+		Resolver:  netxlite.NewStdlibResolver(model.DiscardLogger),
 		UserAgent: "ooniprobe-engine/0.1.0",
 	}).doWithCustomFunc(ctx, invalidIPLookup)
 	if !errors.Is(err, ErrInvalidIPAddress) {

diff --git a/internal/engine/netx/dnstransport.go b/internal/engine/netx/dnstransport.go
@@ -65,7 +65,7 @@ func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride,
 	}
 	switch resolverURL.Scheme {
 	case "system":
-		return netxlite.NewResolverSystem(), nil
+		return netxlite.NewUnwrappedStdlibResolver(), nil
 	case "https":
 		config.TLSConfig.NextProtos = []string{"h2", "http/1.1"}
 		httpClient := &http.Client{Transport: NewHTTPTransport(config)}

diff --git a/internal/engine/netx/resolver.go b/internal/engine/netx/resolver.go
@@ -12,7 +12,7 @@ import (
 // NewResolver creates a new resolver from the specified config.
 func NewResolver(config Config) model.Resolver {
 	if config.BaseResolver == nil {
-		config.BaseResolver = netxlite.NewResolverSystem()
+		config.BaseResolver = netxlite.NewUnwrappedStdlibResolver()
 	}
 	r := netxlite.WrapResolver(
 		model.ValidLoggerOrDefault(config.Logger),

diff --git a/internal/measurex/resolver.go b/internal/measurex/resolver.go
@@ -28,7 +28,7 @@ func WrapResolver(begin time.Time, db WritableDB, r model.Resolver) model.Resolv
 // NewResolverSystem creates a system resolver and then wraps
 // it using the WrapResolver function.
 func (mx *Measurer) NewResolverSystem(db WritableDB, logger model.Logger) model.Resolver {
-	return mx.WrapResolver(db, netxlite.NewResolverStdlib(logger))
+	return mx.WrapResolver(db, netxlite.NewStdlibResolver(logger))
 }
 
 // NewResolverUDP is a convenience factory for creating a Resolver

diff --git a/internal/measurex/tracing.go b/internal/measurex/tracing.go
@@ -54,7 +54,7 @@ func NewTracingHTTPTransport(logger model.Logger, begin time.Time, db WritableDB
 func NewTracingHTTPTransportWithDefaultSettings(
 	begin time.Time, logger model.Logger, db WritableDB) *HTTPTransportDB {
 	return NewTracingHTTPTransport(logger, begin, db,
-		netxlite.NewResolverStdlib(logger),
+		netxlite.NewStdlibResolver(logger),
 		netxlite.NewDialerWithoutResolver(logger),
 		netxlite.NewTLSHandshakerStdlib(logger),
 		DefaultHTTPMaxBodySnapshotSize)

diff --git a/internal/netxlite/dialer.go b/internal/netxlite/dialer.go
@@ -15,10 +15,10 @@ import (
 )
 
 // NewDialerWithStdlibResolver is equivalent to creating a system resolver
-// using NewResolverStdlib and then a dialer using NewDialerWithResolver where
+// using NewStdlibResolver and then a dialer using NewDialerWithResolver where
 // the resolver argument is the previously created resolver.
 func NewDialerWithStdlibResolver(dl model.DebugLogger) model.Dialer {
-	reso := NewResolverStdlib(dl)
+	reso := NewStdlibResolver(dl)
 	return NewDialerWithResolver(dl, reso)
 }
 

diff --git a/internal/netxlite/dialer_test.go b/internal/netxlite/dialer_test.go
@@ -141,7 +141,7 @@ func TestDialerResolver(t *testing.T) {
 		t.Run("fails without a port", func(t *testing.T) {
 			d := &dialerResolver{
 				Dialer:   &DialerSystem{},
-				Resolver: newResolverSystem(),
+				Resolver: NewUnwrappedStdlibResolver(),
 			}
 			const missingPort = "ooni.nu"
 			conn, err := d.DialContext(context.Background(), "tcp", missingPort)

diff --git a/internal/netxlite/http.go b/internal/netxlite/http.go
@@ -323,7 +323,7 @@ func (c *httpTLSConnWithReadTimeout) Read(b []byte) (int, error) {
 // This factory and NewHTTPTransport are the recommended
 // ways of creating a new HTTPTransport.
 func NewHTTPTransportStdlib(logger model.DebugLogger) model.HTTPTransport {
-	dialer := NewDialerWithResolver(logger, NewResolverStdlib(logger))
+	dialer := NewDialerWithResolver(logger, NewStdlibResolver(logger))
 	tlsDialer := NewTLSDialer(dialer, NewTLSHandshakerStdlib(logger))
 	return NewHTTPTransport(logger, dialer, tlsDialer)
 }

diff --git a/internal/netxlite/http_test.go b/internal/netxlite/http_test.go
@@ -236,7 +236,7 @@ func TestNewHTTPTransport(t *testing.T) {
 					called.Add(1)
 				},
 			},
-			Resolver: NewResolverStdlib(log.Log),
+			Resolver: NewStdlibResolver(log.Log),
 		}
 		td := NewTLSDialer(d, NewTLSHandshakerStdlib(log.Log))
 		txp := NewHTTPTransport(log.Log, d, td)

diff --git a/internal/netxlite/integration_test.go b/internal/netxlite/integration_test.go
@@ -41,7 +41,7 @@ func TestMeasureWithSystemResolver(t *testing.T) {
 	//
 
 	t.Run("on success", func(t *testing.T) {
-		r := netxlite.NewResolverStdlib(log.Log)
+		r := netxlite.NewStdlibResolver(log.Log)
 		defer r.CloseIdleConnections()
 		ctx := context.Background()
 		addrs, err := r.LookupHost(ctx, "dns.google.com")
@@ -54,7 +54,7 @@ func TestMeasureWithSystemResolver(t *testing.T) {
 	})
 
 	t.Run("for nxdomain", func(t *testing.T) {
-		r := netxlite.NewResolverStdlib(log.Log)
+		r := netxlite.NewStdlibResolver(log.Log)
 		defer r.CloseIdleConnections()
 		ctx := context.Background()
 		addrs, err := r.LookupHost(ctx, "antani.ooni.org")
@@ -67,7 +67,7 @@ func TestMeasureWithSystemResolver(t *testing.T) {
 	})
 
 	t.Run("for timeout", func(t *testing.T) {
-		r := netxlite.NewResolverStdlib(log.Log)
+		r := netxlite.NewStdlibResolver(log.Log)
 		defer r.CloseIdleConnections()
 		const timeout = time.Nanosecond
 		ctx, cancel := context.WithTimeout(context.Background(), timeout)
@@ -472,7 +472,7 @@ func TestHTTPTransport(t *testing.T) {
 	}
 
 	t.Run("works as intended", func(t *testing.T) {
-		d := netxlite.NewDialerWithResolver(log.Log, netxlite.NewResolverStdlib(log.Log))
+		d := netxlite.NewDialerWithResolver(log.Log, netxlite.NewStdlibResolver(log.Log))
 		td := netxlite.NewTLSDialer(d, netxlite.NewTLSHandshakerStdlib(log.Log))
 		txp := netxlite.NewHTTPTransport(log.Log, d, td)
 		client := &http.Client{Transport: txp}
@@ -523,7 +523,7 @@ func TestHTTP3Transport(t *testing.T) {
 		d := netxlite.NewQUICDialerWithResolver(
 			netxlite.NewQUICListener(),
 			log.Log,
-			netxlite.NewResolverStdlib(log.Log),
+			netxlite.NewStdlibResolver(log.Log),
 		)
 		txp := netxlite.NewHTTP3Transport(log.Log, d, &tls.Config{})
 		client := &http.Client{Transport: txp}

diff --git a/internal/netxlite/legacy.go b/internal/netxlite/legacy.go
diff --git a/internal/netxlite/quic_test.go b/internal/netxlite/quic_test.go
@@ -486,7 +486,7 @@ func TestQUICDialerResolver(t *testing.T) {
 		t.Run("with missing port", func(t *testing.T) {
 			tlsConfig := &tls.Config{}
 			dialer := &quicDialerResolver{
-				Resolver: NewResolverStdlib(log.Log),
+				Resolver: NewStdlibResolver(log.Log),
 				Dialer:   &quicDialerQUICGo{}}
 			qconn, err := dialer.DialContext(
 				context.Background(), "udp", "www.google.com",
@@ -523,7 +523,7 @@ func TestQUICDialerResolver(t *testing.T) {
 			// to establish a connection leads to a failure
 			tlsConf := &tls.Config{}
 			dialer := &quicDialerResolver{
-				Resolver: NewResolverStdlib(log.Log),
+				Resolver: NewStdlibResolver(log.Log),
 				Dialer: &quicDialerQUICGo{
 					QUICListener: &quicListenerStdlib{},
 				}}
@@ -546,7 +546,7 @@ func TestQUICDialerResolver(t *testing.T) {
 			var gotTLSConfig *tls.Config
 			tlsConfig := &tls.Config{}
 			dialer := &quicDialerResolver{
-				Resolver: NewResolverStdlib(log.Log),
+				Resolver: NewStdlibResolver(log.Log),
 				Dialer: &mocks.QUICDialer{
 					MockDialContext: func(ctx context.Context, network, address string,
 						tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlyConnection, error) {
@@ -574,7 +574,7 @@ func TestQUICDialerResolver(t *testing.T) {
 		t.Run("on success", func(t *testing.T) {
 			expectedQConn := &mocks.QUICEarlyConnection{}
 			dialer := &quicDialerResolver{
-				Resolver: NewResolverStdlib(log.Log),
+				Resolver: NewStdlibResolver(log.Log),
 				Dialer: &mocks.QUICDialer{
 					MockDialContext: func(ctx context.Context, network, address string,
 						tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlyConnection, error) {

diff --git a/internal/netxlite/resolvercore.go b/internal/netxlite/resolvercore.go
@@ -23,12 +23,12 @@ import (
 // but you are using the "system" resolver instead.
 var ErrNoDNSTransport = errors.New("operation requires a DNS transport")
 
-// NewResolverStdlib creates a new Resolver by combining WrapResolver
+// NewStdlibResolver creates a new Resolver by combining WrapResolver
 // with an internal "system" resolver type. The list of optional wrappers
 // allow to wrap the underlying getaddrinfo transport. Any nil wrapper
 // will be silently ignored by the code that performs the wrapping.
-func NewResolverStdlib(logger model.DebugLogger, wrappers ...model.DNSTransportWrapper) model.Resolver {
-	return WrapResolver(logger, newResolverSystem(wrappers...))
+func NewStdlibResolver(logger model.DebugLogger, wrappers ...model.DNSTransportWrapper) model.Resolver {
+	return WrapResolver(logger, NewUnwrappedStdlibResolver(wrappers...))
 }
 
 // NewParallelDNSOverHTTPSResolver creates a new DNS over HTTPS resolver
@@ -40,7 +40,10 @@ func NewParallelDNSOverHTTPSResolver(logger model.DebugLogger, URL string) model
 	return WrapResolver(logger, NewUnwrappedParallelResolver(txp))
 }
 
-func newResolverSystem(wrappers ...model.DNSTransportWrapper) *resolverSystem {
+// NewUnwrappedStdlibResolver returns a new, unwrapped resolver using the standard
+// library (i.e., getaddrinfo if possible and &net.Resolver{} otherwise). As the name
+// implies, this function returns an unwrapped resolver.
+func NewUnwrappedStdlibResolver(wrappers ...model.DNSTransportWrapper) model.Resolver {
 	return &resolverSystem{
 		t: WrapDNSTransport(&dnsOverGetaddrinfoTransport{}, wrappers...),
 	}

diff --git a/internal/netxlite/resolvercore_test.go b/internal/netxlite/resolvercore_test.go
@@ -29,7 +29,7 @@ func typecheckForSystemResolver(t *testing.T, resolver model.Resolver, logger mo
 }
 
 func TestNewResolverSystem(t *testing.T) {
-	resolver := NewResolverStdlib(model.DiscardLogger)
+	resolver := NewStdlibResolver(model.DiscardLogger)
 	typecheckForSystemResolver(t, resolver, model.DiscardLogger)
 }
 

diff --git a/internal/tutorial/netxlite/chapter05/README.md b/internal/tutorial/netxlite/chapter05/README.md
@@ -51,7 +51,7 @@ The returned resolver implements an interface that is very
 close to the API of the `net.Resolver` struct.
 
 ```Go
-	reso := netxlite.NewResolverStdlib(log.Log)
+	reso := netxlite.NewStdlibResolver(log.Log)
 ```
 
 We call `LookupHost` to map the hostname to IP addrs. The returned

diff --git a/internal/tutorial/netxlite/chapter05/main.go b/internal/tutorial/netxlite/chapter05/main.go
@@ -52,7 +52,7 @@ func main() {
 	// close to the API of the `net.Resolver` struct.
 	//
 	// ```Go
-	reso := netxlite.NewResolverStdlib(log.Log)
+	reso := netxlite.NewStdlibResolver(log.Log)
 	// ```
 	//
 	// We call `LookupHost` to map the hostname to IP addrs. The returned