From 7c4588716fb7c1ddbecac7e2af13c075b14c4a27 Mon Sep 17 00:00:00 2001
From: Simone Basso <bassosimone@gmail.com>
Date: Wed, 11 Oct 2023 14:25:23 +0200
Subject: [PATCH] [backport] cleanup(riseupvpn): remove summary keys

This diff backports #1360 to the release/3.19 branch.

Removing the summary keys was discussed in
https://github.com/ooni/probe-cli/pull/1125 and I'm extracting this diff
form such a pull request.

While there, bump the version number for this and other upcoming
changes.

---------

Co-authored-by: cyBerta <cyberta@riseup.net>
---
 internal/experiment/riseupvpn/riseupvpn.go    |  20 +---
 .../experiment/riseupvpn/riseupvpn_test.go    | 100 ------------------
 2 files changed, 1 insertion(+), 119 deletions(-)

diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go
index c1b7191093..ba727da906 100644
--- a/internal/experiment/riseupvpn/riseupvpn.go
+++ b/internal/experiment/riseupvpn/riseupvpn.go
@@ -6,7 +6,6 @@ package riseupvpn
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"time"
 
 	"github.com/ooni/probe-cli/v3/internal/experiment/urlgetter"
@@ -333,28 +332,11 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer {
 // Note that this structure is part of the ABI contract with ooniprobe
 // therefore we should be careful when changing it.
 type SummaryKeys struct {
-	APIBlocked      bool              `json:"api_blocked"`
-	ValidCACert     bool              `json:"valid_ca_cert"`
-	FailingGateways int               `json:"failing_gateways"`
-	TransportStatus map[string]string `json:"transport_status"`
-	IsAnomaly       bool              `json:"-"`
+	IsAnomaly bool `json:"-"`
 }
 
 // GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys.
 func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) {
 	sk := SummaryKeys{IsAnomaly: false}
-	tk, ok := measurement.TestKeys.(*TestKeys)
-	if !ok {
-		return sk, errors.New("invalid test keys type")
-	}
-	sk.APIBlocked = tk.APIStatus != "ok"
-	sk.ValidCACert = tk.CACertStatus
-	sk.FailingGateways = len(tk.FailingGateways)
-	sk.TransportStatus = tk.TransportStatus
-	// Note: the order in the following OR chains matter: TransportStatus
-	// is nil if APIBlocked or !CACertStatus
-	sk.IsAnomaly = (sk.APIBlocked || !tk.CACertStatus ||
-		tk.TransportStatus["openvpn"] == "blocked" ||
-		tk.TransportStatus["obfs4"] == "blocked")
 	return sk, nil
 }
diff --git a/internal/experiment/riseupvpn/riseupvpn_test.go b/internal/experiment/riseupvpn/riseupvpn_test.go
index aee79313c5..80e956d593 100644
--- a/internal/experiment/riseupvpn/riseupvpn_test.go
+++ b/internal/experiment/riseupvpn/riseupvpn_test.go
@@ -11,7 +11,6 @@ import (
 	"testing"
 
 	"github.com/apex/log"
-	"github.com/google/go-cmp/cmp"
 	"github.com/ooni/probe-cli/v3/internal/experiment/riseupvpn"
 	"github.com/ooni/probe-cli/v3/internal/experiment/urlgetter"
 	"github.com/ooni/probe-cli/v3/internal/legacy/mockable"
@@ -623,105 +622,6 @@ func TestMissingTransport(t *testing.T) {
 	}
 }
 
-func TestSummaryKeysInvalidType(t *testing.T) {
-	measurement := new(model.Measurement)
-	m := &riseupvpn.Measurer{}
-	_, err := m.GetSummaryKeys(measurement)
-	if err.Error() != "invalid test keys type" {
-		t.Fatal("not the error we expected")
-	}
-}
-
-func TestSummaryKeysWorksAsIntended(t *testing.T) {
-	tests := []struct {
-		tk riseupvpn.TestKeys
-		sk riseupvpn.SummaryKeys
-	}{{
-		tk: riseupvpn.TestKeys{
-			APIStatus:       "blocked",
-			CACertStatus:    true,
-			FailingGateways: nil,
-			TransportStatus: nil,
-		},
-		sk: riseupvpn.SummaryKeys{
-			APIBlocked:      true,
-			ValidCACert:     true,
-			IsAnomaly:       true,
-			TransportStatus: nil,
-			FailingGateways: 0,
-		},
-	}, {
-		tk: riseupvpn.TestKeys{
-			APIStatus:       "ok",
-			CACertStatus:    false,
-			FailingGateways: nil,
-			TransportStatus: nil,
-		},
-		sk: riseupvpn.SummaryKeys{
-			ValidCACert:     false,
-			IsAnomaly:       true,
-			FailingGateways: 0,
-			TransportStatus: nil,
-		},
-	}, {
-		tk: riseupvpn.TestKeys{
-			APIStatus:    "ok",
-			CACertStatus: true,
-			FailingGateways: []riseupvpn.GatewayConnection{{
-				IP:            "1.1.1.1",
-				Port:          443,
-				TransportType: "obfs4",
-			}},
-			TransportStatus: map[string]string{
-				"obfs4":   "blocked",
-				"openvpn": "ok",
-			},
-		},
-		sk: riseupvpn.SummaryKeys{
-			FailingGateways: 1,
-			IsAnomaly:       true,
-			ValidCACert:     true,
-			TransportStatus: map[string]string{
-				"obfs4":   "blocked",
-				"openvpn": "ok",
-			},
-		},
-	}, {
-		tk: riseupvpn.TestKeys{
-			APIStatus:       "ok",
-			CACertStatus:    true,
-			FailingGateways: nil,
-			TransportStatus: map[string]string{
-				"openvpn": "ok",
-			},
-		},
-		sk: riseupvpn.SummaryKeys{
-			ValidCACert:     true,
-			IsAnomaly:       false,
-			FailingGateways: 0,
-			TransportStatus: map[string]string{
-				"openvpn": "ok",
-			},
-		},
-	},
-	}
-	for idx, tt := range tests {
-		t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) {
-			m := &riseupvpn.Measurer{}
-			measurement := &model.Measurement{TestKeys: &tt.tk}
-			got, err := m.GetSummaryKeys(measurement)
-			if err != nil {
-				t.Fatal(err)
-				return
-			}
-			sk := got.(riseupvpn.SummaryKeys)
-			if diff := cmp.Diff(tt.sk, sk); diff != "" {
-				t.Fatal(diff)
-			}
-		})
-	}
-}
-
 func generateMockGetter(requestResponse map[string]string, responseStatus map[string]bool) urlgetter.MultiGetter {
 	return func(ctx context.Context, g urlgetter.Getter) (urlgetter.TestKeys, error) {
 		url := g.Target