cleanup(webconnectivitytle): avoid code duplication (#1456)

Use a single algorithm for determining whether there's HTTP diff. We can
do this by migrating the HTTP diff flags to use `optional.Value` and
then we can always use code written for the "ext" sub-engine to do that.

Part of ooni/probe#2640

internal/experiment/webconnectivitylte/analysisclassic.go

@@ -83,28 +83,12 @@ func analysisClassicDNSConsistency(woa *minipipeline.WebAnalysis) optional.Value
 }
 
 func (tk *TestKeys) setHTTPDiffValues(woa *minipipeline.WebAnalysis) {
-	// TODO(bassosimone): this code should use [newAnalysisHTTPDiffStatus].
-	const bodyProportionFactor = 0.7
-	if !woa.HTTPFinalResponseDiffBodyProportionFactor.IsNone() {
-		tk.BodyProportion = woa.HTTPFinalResponseDiffBodyProportionFactor.Unwrap()
-		value := tk.BodyProportion > bodyProportionFactor
-		tk.BodyLengthMatch = &value
-	}
-
-	if !woa.HTTPFinalResponseDiffUncommonHeadersIntersection.IsNone() {
-		value := len(woa.HTTPFinalResponseDiffUncommonHeadersIntersection.Unwrap()) > 0
-		tk.HeadersMatch = &value
-	}
-
-	if !woa.HTTPFinalResponseDiffStatusCodeMatch.IsNone() {
-		value := woa.HTTPFinalResponseDiffStatusCodeMatch.Unwrap()
-		tk.StatusCodeMatch = &value
-	}
-
-	if !woa.HTTPFinalResponseDiffTitleDifferentLongWords.IsNone() {
-		value := len(woa.HTTPFinalResponseDiffTitleDifferentLongWords.Unwrap()) <= 0
-		tk.TitleMatch = &value
-	}
+	hds := newAnalysisHTTPDiffStatus(woa)
+	tk.BodyProportion = hds.BodyProportion.UnwrapOr(0)
+	tk.BodyLengthMatch = hds.BodyLengthMatch
+	tk.HeadersMatch = hds.HeadersMatch
+	tk.StatusCodeMatch = hds.StatusCodeMatch
+	tk.TitleMatch = hds.TitleMatch
 }
 
 type analysisClassicTestKeysProxy interface {
@@ -128,20 +112,27 @@ var _ analysisClassicTestKeysProxy = &TestKeys{}
 
 // httpDiff implements analysisClassicTestKeysProxy.
 func (tk *TestKeys) httpDiff() bool {
-	// TODO(bassosimone): this code should use [newAnalysisHTTPDiffStatus].
-	if tk.StatusCodeMatch != nil && *tk.StatusCodeMatch {
-		if tk.BodyLengthMatch != nil && *tk.BodyLengthMatch {
-			return false
-		}
-		if tk.HeadersMatch != nil && *tk.HeadersMatch {
-			return false
-		}
-		if tk.TitleMatch != nil && *tk.TitleMatch {
-			return false
-		}
-		// fallthrough
-	}
-	return true
+	return analysisHTTPDiffAlgorithm(tk)
+}
+
+// bodyLengthMatch implements analysisHTTPDiffValuesProvider.
+func (tk *TestKeys) bodyLengthMatch() optional.Value[bool] {
+	return tk.BodyLengthMatch
+}
+
+// headersMatch implements analysisHTTPDiffValuesProvider.
+func (tk *TestKeys) headersMatch() optional.Value[bool] {
+	return tk.HeadersMatch
+}
+
+// statusCodeMatch implements analysisHTTPDiffValuesProvider.
+func (tk *TestKeys) statusCodeMatch() optional.Value[bool] {
+	return tk.StatusCodeMatch
+}
+
+// titleMatch implements analysisHTTPDiffValuesProvider.
+func (tk *TestKeys) titleMatch() optional.Value[bool] {
+	return tk.TitleMatch
 }
 
 // setBlockingFalse implements analysisClassicTestKeysProxy.

internal/experiment/webconnectivitylte/analysisext.go

@@ -142,7 +142,7 @@ func analysisExtHTTPFinalResponse(tk *TestKeys, analysis *minipipeline.WebAnalys
 	if success := analysis.HTTPFinalResponseSuccessTCPWithControl; !success.IsNone() {
 		txID := success.Unwrap()
 		hds := newAnalysisHTTPDiffStatus(analysis)
-		if hds.httpDiff() {
+		if analysisHTTPDiffAlgorithm(hds) {
 			tk.BlockingFlags |= AnalysisBlockingFlagHTTPDiff
 			fmt.Fprintf(info, "- the final response (transaction: %d) differs from the control response\n", txID)
 			return

internal/experiment/webconnectivitylte/analysishttpdiff.go

@@ -51,16 +51,45 @@ func newAnalysisHTTPDiffStatus(analysis *minipipeline.WebAnalysis) *analysisHTTP
 	return hds
 }
 
-// httpDiff computes whether there is HTTP diff.
-func (hds *analysisHTTPDiffStatus) httpDiff() bool {
-	if !hds.StatusCodeMatch.IsNone() && hds.StatusCodeMatch.Unwrap() {
-		if !hds.BodyLengthMatch.IsNone() && hds.BodyLengthMatch.Unwrap() {
+type analysisHTTPDiffValuesProvider interface {
+	bodyLengthMatch() optional.Value[bool]
+	headersMatch() optional.Value[bool]
+	statusCodeMatch() optional.Value[bool]
+	titleMatch() optional.Value[bool]
+}
+
+var _ analysisHTTPDiffValuesProvider = &analysisHTTPDiffStatus{}
+
+// bodyLengthMatch implements analysisHTTPDiffValuesProvider.
+func (hds *analysisHTTPDiffStatus) bodyLengthMatch() optional.Value[bool] {
+	return hds.BodyLengthMatch
+}
+
+// headersMatch implements analysisHTTPDiffValuesProvider.
+func (hds *analysisHTTPDiffStatus) headersMatch() optional.Value[bool] {
+	return hds.HeadersMatch
+}
+
+// statusCodeMatch implements analysisHTTPDiffValuesProvider.
+func (hds *analysisHTTPDiffStatus) statusCodeMatch() optional.Value[bool] {
+	return hds.StatusCodeMatch
+}
+
+// titleMatch implements analysisHTTPDiffValuesProvider.
+func (hds *analysisHTTPDiffStatus) titleMatch() optional.Value[bool] {
+	return hds.TitleMatch
+}
+
+// analysisHTTPDiffAlgorithm returns whether there's an HTTP diff
+func analysisHTTPDiffAlgorithm(p analysisHTTPDiffValuesProvider) bool {
+	if !p.statusCodeMatch().IsNone() && p.statusCodeMatch().Unwrap() {
+		if !p.bodyLengthMatch().IsNone() && p.bodyLengthMatch().Unwrap() {
 			return false
 		}
-		if !hds.HeadersMatch.IsNone() && hds.HeadersMatch.Unwrap() {
+		if !p.headersMatch().IsNone() && p.headersMatch().Unwrap() {
 			return false
 		}
-		if !hds.TitleMatch.IsNone() && hds.TitleMatch.Unwrap() {
+		if !p.titleMatch().IsNone() && p.titleMatch().Unwrap() {
 			return false
 		}
 		// fallthrough

internal/experiment/webconnectivitylte/testkeys.go

@@ -99,16 +99,16 @@ type TestKeys struct {
 	BodyProportion float64 `json:"body_proportion"`
 
 	// BodyLength match tells us whether the body length matches.
-	BodyLengthMatch *bool `json:"body_length_match"`
+	BodyLengthMatch optional.Value[bool] `json:"body_length_match"`
 
 	// HeadersMatch tells us whether the headers match.
-	HeadersMatch *bool `json:"headers_match"`
+	HeadersMatch optional.Value[bool] `json:"headers_match"`
 
 	// StatusCodeMatch tells us whether the status code matches.
-	StatusCodeMatch *bool `json:"status_code_match"`
+	StatusCodeMatch optional.Value[bool] `json:"status_code_match"`
 
 	// TitleMatch tells us whether the title matches.
-	TitleMatch *bool `json:"title_match"`
+	TitleMatch optional.Value[bool] `json:"title_match"`
 
 	// Blocking indicates the reason for blocking. This is notoriously a bad
 	// type because it can be one of the following values:
@@ -363,10 +363,10 @@ func NewTestKeys() *TestKeys {
 		BlockingFlags:         0,
 		NullNullFlags:         0,
 		BodyProportion:        0,
-		BodyLengthMatch:       nil,
-		HeadersMatch:          nil,
-		StatusCodeMatch:       nil,
-		TitleMatch:            nil,
+		BodyLengthMatch:       optional.None[bool](),
+		HeadersMatch:          optional.None[bool](),
+		StatusCodeMatch:       optional.None[bool](),
+		TitleMatch:            optional.None[bool](),
 		Blocking:              nil,
 		Accessible:            nil,
 		ControlRequest:        nil,