Skip to content

Commit 7c45887

Browse files
bassosimonecyBerta
andcommitted
[backport] cleanup(riseupvpn): remove summary keys
This diff backports #1360 to the release/3.19 branch. Removing the summary keys was discussed in #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 <[email protected]>
1 parent c278dfd commit 7c45887

File tree

2 files changed

+1
-119
lines changed

2 files changed

+1
-119
lines changed

internal/experiment/riseupvpn/riseupvpn.go

+1-19
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package riseupvpn
66
import (
77
"context"
88
"encoding/json"
9-
"errors"
109
"time"
1110

1211
"github.com/ooni/probe-cli/v3/internal/experiment/urlgetter"
@@ -333,28 +332,11 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer {
333332
// Note that this structure is part of the ABI contract with ooniprobe
334333
// therefore we should be careful when changing it.
335334
type SummaryKeys struct {
336-
APIBlocked bool `json:"api_blocked"`
337-
ValidCACert bool `json:"valid_ca_cert"`
338-
FailingGateways int `json:"failing_gateways"`
339-
TransportStatus map[string]string `json:"transport_status"`
340-
IsAnomaly bool `json:"-"`
335+
IsAnomaly bool `json:"-"`
341336
}
342337

343338
// GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys.
344339
func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) {
345340
sk := SummaryKeys{IsAnomaly: false}
346-
tk, ok := measurement.TestKeys.(*TestKeys)
347-
if !ok {
348-
return sk, errors.New("invalid test keys type")
349-
}
350-
sk.APIBlocked = tk.APIStatus != "ok"
351-
sk.ValidCACert = tk.CACertStatus
352-
sk.FailingGateways = len(tk.FailingGateways)
353-
sk.TransportStatus = tk.TransportStatus
354-
// Note: the order in the following OR chains matter: TransportStatus
355-
// is nil if APIBlocked or !CACertStatus
356-
sk.IsAnomaly = (sk.APIBlocked || !tk.CACertStatus ||
357-
tk.TransportStatus["openvpn"] == "blocked" ||
358-
tk.TransportStatus["obfs4"] == "blocked")
359341
return sk, nil
360342
}

internal/experiment/riseupvpn/riseupvpn_test.go

-100
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"testing"
1212

1313
"github.com/apex/log"
14-
"github.com/google/go-cmp/cmp"
1514
"github.com/ooni/probe-cli/v3/internal/experiment/riseupvpn"
1615
"github.com/ooni/probe-cli/v3/internal/experiment/urlgetter"
1716
"github.com/ooni/probe-cli/v3/internal/legacy/mockable"
@@ -623,105 +622,6 @@ func TestMissingTransport(t *testing.T) {
623622
}
624623
}
625624

626-
func TestSummaryKeysInvalidType(t *testing.T) {
627-
measurement := new(model.Measurement)
628-
m := &riseupvpn.Measurer{}
629-
_, err := m.GetSummaryKeys(measurement)
630-
if err.Error() != "invalid test keys type" {
631-
t.Fatal("not the error we expected")
632-
}
633-
}
634-
635-
func TestSummaryKeysWorksAsIntended(t *testing.T) {
636-
tests := []struct {
637-
tk riseupvpn.TestKeys
638-
sk riseupvpn.SummaryKeys
639-
}{{
640-
tk: riseupvpn.TestKeys{
641-
APIStatus: "blocked",
642-
CACertStatus: true,
643-
FailingGateways: nil,
644-
TransportStatus: nil,
645-
},
646-
sk: riseupvpn.SummaryKeys{
647-
APIBlocked: true,
648-
ValidCACert: true,
649-
IsAnomaly: true,
650-
TransportStatus: nil,
651-
FailingGateways: 0,
652-
},
653-
}, {
654-
tk: riseupvpn.TestKeys{
655-
APIStatus: "ok",
656-
CACertStatus: false,
657-
FailingGateways: nil,
658-
TransportStatus: nil,
659-
},
660-
sk: riseupvpn.SummaryKeys{
661-
ValidCACert: false,
662-
IsAnomaly: true,
663-
FailingGateways: 0,
664-
TransportStatus: nil,
665-
},
666-
}, {
667-
tk: riseupvpn.TestKeys{
668-
APIStatus: "ok",
669-
CACertStatus: true,
670-
FailingGateways: []riseupvpn.GatewayConnection{{
671-
IP: "1.1.1.1",
672-
Port: 443,
673-
TransportType: "obfs4",
674-
}},
675-
TransportStatus: map[string]string{
676-
"obfs4": "blocked",
677-
"openvpn": "ok",
678-
},
679-
},
680-
sk: riseupvpn.SummaryKeys{
681-
FailingGateways: 1,
682-
IsAnomaly: true,
683-
ValidCACert: true,
684-
TransportStatus: map[string]string{
685-
"obfs4": "blocked",
686-
"openvpn": "ok",
687-
},
688-
},
689-
}, {
690-
tk: riseupvpn.TestKeys{
691-
APIStatus: "ok",
692-
CACertStatus: true,
693-
FailingGateways: nil,
694-
TransportStatus: map[string]string{
695-
"openvpn": "ok",
696-
},
697-
},
698-
sk: riseupvpn.SummaryKeys{
699-
ValidCACert: true,
700-
IsAnomaly: false,
701-
FailingGateways: 0,
702-
TransportStatus: map[string]string{
703-
"openvpn": "ok",
704-
},
705-
},
706-
},
707-
}
708-
for idx, tt := range tests {
709-
t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) {
710-
m := &riseupvpn.Measurer{}
711-
measurement := &model.Measurement{TestKeys: &tt.tk}
712-
got, err := m.GetSummaryKeys(measurement)
713-
if err != nil {
714-
t.Fatal(err)
715-
return
716-
}
717-
sk := got.(riseupvpn.SummaryKeys)
718-
if diff := cmp.Diff(tt.sk, sk); diff != "" {
719-
t.Fatal(diff)
720-
}
721-
})
722-
}
723-
}
724-
725625
func generateMockGetter(requestResponse map[string]string, responseStatus map[string]bool) urlgetter.MultiGetter {
726626
return func(ctx context.Context, g urlgetter.Getter) (urlgetter.TestKeys, error) {
727627
url := g.Target

0 commit comments

Comments
 (0)