fix: message format and testing

ooni · Jan 25, 2021 · 18c8bec · 18c8bec
1 parent bb28592
commit 18c8bec
Show file tree

Hide file tree

Showing 4 changed files with 134 additions and 15 deletions.
diff --git a/model/checkininfo.go b/model/checkininfo.go
@@ -2,11 +2,11 @@ package model
 
 // CheckInInfoWebConnectivity contains the array of URLs returned by the checkin API
 type CheckInInfoWebConnectivity struct {
-	URLs []URLInfo `json:"urls"`
+	ReportID string    `json:"report_id"`
+	URLs     []URLInfo `json:"urls"`
 }
 
 // CheckInInfo contains the return test objects from the checkin API
 type CheckInInfo struct {
-	ReportID        string                      `json:"report_id"`
 	WebConnectivity *CheckInInfoWebConnectivity `json:"web_connectivity"`
 }
diff --git a/oonimkall/session.go b/oonimkall/session.go
@@ -97,6 +97,10 @@ type Session struct {
 	mtx       sync.Mutex
 	submitter *probeservices.Submitter
 	sessp     *engine.Session
+
+	// Hooks for testing (should not appear in Java/ObjC)
+	TestingCheckInBeforeNewProbeServicesClient func(ctx *Context)
+	TestingCheckInBeforeCheckIn                func(ctx *Context)
 }
 
 // NewSession creates a new session. You should use a session for running
@@ -286,7 +290,8 @@ type CheckInConfig struct {
 
 // CheckInInfoWebConnectivity contains the array of URLs returned by the checkin API
 type CheckInInfoWebConnectivity struct {
-	URLs []model.URLInfo
+	ReportID string
+	URLs     []model.URLInfo
 }
 
 // URLInfo contains info on a test lists URL
@@ -296,8 +301,13 @@ type URLInfo struct {
 	URL          string
 }
 
-// Get the URLInfo at position idx from CheckInInfoWebConnectivity.URLs
-func (ckw *CheckInInfoWebConnectivity) Get(idx int64) *URLInfo {
+// Size returns the number of URLs.
+func (ckw *CheckInInfoWebConnectivity) Size() int64 {
+	return int64(len(ckw.URLs))
+}
+
+// At gets the URLInfo at position idx from CheckInInfoWebConnectivity.URLs
+func (ckw *CheckInInfoWebConnectivity) At(idx int64) *URLInfo {
 	if idx < 0 || int(idx) >= len(ckw.URLs) {
 		return nil
 	}
@@ -309,15 +319,17 @@ func (ckw *CheckInInfoWebConnectivity) Get(idx int64) *URLInfo {
 	}
 }
 
-func newCheckInInfoWebConnectivity(ckw model.CheckInInfoWebConnectivity) *CheckInInfoWebConnectivity {
-	return &CheckInInfoWebConnectivity{
-		URLs: ckw.URLs,
+func newCheckInInfoWebConnectivity(ckw *model.CheckInInfoWebConnectivity) *CheckInInfoWebConnectivity {
+	out := new(CheckInInfoWebConnectivity)
+	if ckw != nil {
+		out.ReportID = ckw.ReportID
+		out.URLs = ckw.URLs
 	}
+	return out
 }
 
 // CheckInInfo contains the return test objects from the checkin API
 type CheckInInfo struct {
-	ReportID        string
 	WebConnectivity *CheckInInfoWebConnectivity
 }
 
@@ -334,10 +346,16 @@ func (sess *Session) CheckIn(ctx *Context, config *CheckInConfig) (*CheckInInfo,
 	if err != nil {
 		return nil, err
 	}
+	if sess.TestingCheckInBeforeNewProbeServicesClient != nil {
+		sess.TestingCheckInBeforeNewProbeServicesClient(ctx)
+	}
 	psc, err := sess.sessp.NewProbeServicesClient(ctx.ctx)
 	if err != nil {
 		return nil, err
 	}
+	if sess.TestingCheckInBeforeCheckIn != nil {
+		sess.TestingCheckInBeforeCheckIn(ctx)
+	}
 	cfg := model.CheckInConfig{
 		Charging:        config.Charging,
 		OnWiFi:          config.OnWiFi,
@@ -353,7 +371,6 @@ func (sess *Session) CheckIn(ctx *Context, config *CheckInConfig) (*CheckInInfo,
 		return nil, err
 	}
 	return &CheckInInfo{
-		ReportID:        result.ReportID,
-		WebConnectivity: newCheckInInfoWebConnectivity(*result.WebConnectivity),
+		WebConnectivity: newCheckInInfoWebConnectivity(result.WebConnectivity),
 	}, nil
 }
diff --git a/oonimkall/session_integration_test.go b/oonimkall/session_integration_test.go
@@ -12,6 +12,7 @@ import (
 	"testing"
 	"time"
 
+	engine "github.com/ooni/probe-engine"
 	"github.com/ooni/probe-engine/geolocate"
 	"github.com/ooni/probe-engine/model"
 	"github.com/ooni/probe-engine/oonimkall"
@@ -280,21 +281,119 @@ func TestCheckInSuccess(t *testing.T) {
 	config.WebConnectivity.Add("NEWS")
 	config.WebConnectivity.Add("CULTR")
 	result, err := sess.CheckIn(ctx, &config)
-
 	if err != nil {
 		t.Fatalf("unexpected error: %+v", err)
 	}
 	if result == nil || result.WebConnectivity == nil {
 		t.Fatal("got nil result or WebConnectivity")
 	}
 	if len(result.WebConnectivity.URLs) < 1 {
-		t.Fatal("unexpected number of results")
+		t.Fatal("unexpected number of URLs")
+	}
+	if result.WebConnectivity.ReportID == "" {
+		t.Fatal("got empty report ID")
+	}
+	siz := result.WebConnectivity.Size()
+	if siz <= 0 {
+		t.Fatal("unexpected number of URLs")
 	}
-	for _, entry := range result.WebConnectivity.URLs {
+	for idx := int64(0); idx < siz; idx++ {
+		entry := result.WebConnectivity.At(idx)
 		if entry.CategoryCode != "NEWS" && entry.CategoryCode != "CULTR" {
 			t.Fatalf("unexpected category code: %+v", entry)
 		}
 	}
+	if result.WebConnectivity.At(-1) != nil {
+		t.Fatal("expected nil here")
+	}
+	if result.WebConnectivity.At(siz) != nil {
+		t.Fatal("expected nil here")
+	}
+}
+
+func TestCheckInLookupLocationFailure(t *testing.T) {
+	sess, err := NewSession()
+	if err != nil {
+		t.Fatal(err)
+	}
+	ctx := sess.NewContext()
+	config := oonimkall.CheckInConfig{
+		Charging:        true,
+		OnWiFi:          true,
+		Platform:        "android",
+		RunType:         "timed",
+		SoftwareName:    "ooniprobe-android",
+		SoftwareVersion: "2.7.1",
+		WebConnectivity: &oonimkall.CheckInConfigWebConnectivity{},
+	}
+	config.WebConnectivity.Add("NEWS")
+	config.WebConnectivity.Add("CULTR")
+	ctx.Cancel() // immediate failure
+	result, err := sess.CheckIn(ctx, &config)
+	if !errors.Is(err, geolocate.ErrAllIPLookuppersFailed) {
+		t.Fatalf("not the error we expected: %+v", err)
+	}
+	if result != nil {
+		t.Fatal("expected nil result here")
+	}
+}
+
+func TestCheckInNewProbeServicesFailure(t *testing.T) {
+	sess, err := NewSession()
+	if err != nil {
+		t.Fatal(err)
+	}
+	sess.TestingCheckInBeforeNewProbeServicesClient = func(ctx *oonimkall.Context) {
+		ctx.Cancel() // cancel execution
+	}
+	ctx := sess.NewContext()
+	config := oonimkall.CheckInConfig{
+		Charging:        true,
+		OnWiFi:          true,
+		Platform:        "android",
+		RunType:         "timed",
+		SoftwareName:    "ooniprobe-android",
+		SoftwareVersion: "2.7.1",
+		WebConnectivity: &oonimkall.CheckInConfigWebConnectivity{},
+	}
+	config.WebConnectivity.Add("NEWS")
+	config.WebConnectivity.Add("CULTR")
+	result, err := sess.CheckIn(ctx, &config)
+	if !errors.Is(err, engine.ErrAllProbeServicesFailed) {
+		t.Fatalf("not the error we expected: %+v", err)
+	}
+	if result != nil {
+		t.Fatal("expected nil result here")
+	}
+}
+
+func TestCheckInCheckInFailure(t *testing.T) {
+	sess, err := NewSession()
+	if err != nil {
+		t.Fatal(err)
+	}
+	sess.TestingCheckInBeforeCheckIn = func(ctx *oonimkall.Context) {
+		ctx.Cancel() // cancel execution
+	}
+	ctx := sess.NewContext()
+	config := oonimkall.CheckInConfig{
+		Charging:        true,
+		OnWiFi:          true,
+		Platform:        "android",
+		RunType:         "timed",
+		SoftwareName:    "ooniprobe-android",
+		SoftwareVersion: "2.7.1",
+		WebConnectivity: &oonimkall.CheckInConfigWebConnectivity{},
+	}
+	config.WebConnectivity.Add("NEWS")
+	config.WebConnectivity.Add("CULTR")
+	result, err := sess.CheckIn(ctx, &config)
+	if !errors.Is(err, context.Canceled) {
+		t.Fatalf("not the error we expected: %+v", err)
+	}
+	if result != nil {
+		t.Fatal("expected nil result here")
+	}
 }
 
 func TestCheckInNoParams(t *testing.T) {

diff --git a/probeservices/checkin_test.go b/probeservices/checkin_test.go
@@ -32,8 +32,11 @@ func TestCheckInSuccess(t *testing.T) {
 	if result == nil || result.WebConnectivity == nil {
 		t.Fatal("got nil result or WebConnectivity")
 	}
+	if result.WebConnectivity.ReportID == "" {
+		t.Fatal("ReportID is empty")
+	}
 	if len(result.WebConnectivity.URLs) < 1 {
-		t.Fatal("unexpected number of results")
+		t.Fatal("unexpected number of URLs")
 	}
 	for _, entry := range result.WebConnectivity.URLs {
 		if entry.CategoryCode != "NEWS" && entry.CategoryCode != "CULTR" {