Make sure ooniprobe show <id> is WAI (#61)

1. the description of the command and the helper function are
clear hints that the command is intended to show a single JSON
measurement at a time (also the use case seems clear) [*]

2. the function used to read lines was failing for all my
measurements that take input. Since that was not the optimal
pattern anyway, use a better pattern to fix it.

3. some changes are automatically applied by my editor (VSCode
with the Go plugin) and I am fine with them.

4. while reading code, I also applied my preferred pattern
wrt whitespaces, i.e.: no whitespace inside functions, if a
function feels too long in this way, just break it.

Closes #57

[*] Even if we want to show many measurements at a time, which
does not seem needed, given the UI patterns, this functionality
won't be P0. What is P0 is to bless a new beta and give to
@sarathms binaries for all archs that support a basic `show`.
This commit is contained in:
Simone Basso 2019-10-03 11:18:07 +02:00 committed by GitHub
parent f3865d2ec0
commit 7cde93fca4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 17 additions and 49 deletions

View File

@ -10,9 +10,7 @@ import (
func init() {
cmd := root.Command("list", "List results")
resultID := cmd.Arg("id", "the id of the result to list measurements for").Int64()
cmd.Action(func(_ *kingpin.ParseContext) error {
ctx, err := root.Init()
if err != nil {
@ -25,7 +23,6 @@ func init() {
log.WithError(err).Error("failed to list measurements")
return err
}
msmtSummary := output.MeasurementSummaryData{
TotalCount: 0,
AnomalyCount: 0,
@ -45,7 +42,6 @@ func init() {
if idx == len(measurements)-1 {
isLast = true
}
// We assume that since these are summary level information the first
// item will contain the information necessary.
if isFirst {
@ -70,7 +66,6 @@ func init() {
log.WithError(err).Error("failed to list results")
return err
}
if len(incompleteResults) > 0 {
output.SectionTitle("Incomplete results")
}
@ -92,7 +87,6 @@ func init() {
DataUsageDown: result.DataUsageDown,
})
}
resultSummary := output.ResultSummaryData{}
netCount := make(map[uint]int)
output.SectionTitle("Results")
@ -117,9 +111,9 @@ func init() {
TestKeys: testKeys,
MeasurementCount: totalCount,
MeasurementAnomalyCount: anmlyCount,
Done: result.IsDone,
DataUsageUp: result.DataUsageUp,
DataUsageDown: result.DataUsageDown,
Done: result.IsDone,
DataUsageUp: result.DataUsageUp,
DataUsageDown: result.DataUsageDown,
})
resultSummary.TotalTests++
netCount[result.Network.ASN]++
@ -127,10 +121,8 @@ func init() {
resultSummary.TotalDataUsageDown += result.DataUsageDown
}
resultSummary.TotalNetworks = int64(len(netCount))
output.ResultSummary(resultSummary)
}
return nil
})
}

View File

@ -10,9 +10,7 @@ import (
func init() {
cmd := root.Command("show", "Show a specific measurement")
msmtID := cmd.Arg("id", "the id of the measurement to show").Int64()
msmtID := cmd.Arg("id", "the id of the measurement to show").Required().Int64()
cmd.Action(func(_ *kingpin.ParseContext) error {
ctx, err := root.Init()
if err != nil {

View File

@ -1,9 +1,9 @@
package database
import (
"bufio"
"database/sql"
"encoding/json"
"bufio"
"io"
"io/ioutil"
"os"
@ -11,9 +11,8 @@ import (
"time"
"github.com/apex/log"
"github.com/ooni/probe-cli/utils"
"github.com/ooni/probe-cli/internal/enginex"
"github.com/ooni/probe-cli/internal/util"
"github.com/ooni/probe-cli/utils"
"github.com/pkg/errors"
db "upper.io/db.v3"
"upper.io/db.v3/lib/sqlbuilder"
@ -22,7 +21,6 @@ import (
// ListMeasurements given a result ID
func ListMeasurements(sess sqlbuilder.Database, resultID int64) ([]MeasurementURLNetwork, error) {
measurements := []MeasurementURLNetwork{}
req := sess.Select(
db.Raw("networks.*"),
db.Raw("urls.*"),
@ -34,7 +32,6 @@ func ListMeasurements(sess sqlbuilder.Database, resultID int64) ([]MeasurementUR
LeftJoin("urls").On("urls.url_id = measurements.url_id").
OrderBy("measurements.measurement_start_time").
Where("results.result_id = ?", resultID)
if err := req.All(&measurements); err != nil {
log.Errorf("failed to run query %s: %v", req.String(), err)
return measurements, err
@ -42,20 +39,18 @@ func ListMeasurements(sess sqlbuilder.Database, resultID int64) ([]MeasurementUR
return measurements, nil
}
// GetMeasurementJSON will a map[string]interface{} given a database and a measurementID
// GetMeasurementJSON returns a map[string]interface{} given a database and a measurementID
func GetMeasurementJSON(sess sqlbuilder.Database, measurementID int64) (map[string]interface{}, error) {
var (
measurement MeasurementURLNetwork
msmtJSON map[string]interface{}
msmtJSON map[string]interface{}
)
req := sess.Select(
db.Raw("urls.*"),
db.Raw("measurements.*"),
).From("measurements").
LeftJoin("urls").On("urls.url_id = measurements.url_id").
Where("measurements.measurement_id= ?", measurementID)
if err := req.One(&measurement); err != nil {
log.Errorf("failed to run query %s: %v", req.String(), err)
return nil, err
@ -63,7 +58,7 @@ func GetMeasurementJSON(sess sqlbuilder.Database, measurementID int64) (map[stri
reportFilePath := measurement.Measurement.ReportFilePath
// If the url->url is NULL then we are dealing with a single entry
// measurement and all we have to do is read the file and return it.
if (measurement.URL.URL.Valid == false) {
if measurement.URL.URL.Valid == false {
b, err := ioutil.ReadFile(reportFilePath)
if err != nil {
return nil, err
@ -73,7 +68,6 @@ func GetMeasurementJSON(sess sqlbuilder.Database, measurementID int64) (map[stri
}
return msmtJSON, nil
}
// When the URL is a string then we need to seek until we reach the
// measurement line in the file that matches the target input
url := measurement.URL.URL.String
@ -82,20 +76,19 @@ func GetMeasurementJSON(sess sqlbuilder.Database, measurementID int64) (map[stri
return nil, err
}
defer file.Close()
reader := bufio.NewReader(file)
for {
line, err := util.ReadLine(reader)
if (err == io.EOF) {
line, err := reader.ReadString('\n')
if err == io.EOF {
break
} else if err != nil {
}
if err != nil {
return nil, err
}
if err := json.Unmarshal([]byte(line), &msmtJSON); err != nil {
return nil, err
}
if (msmtJSON["input"].(string) == url) {
if msmtJSON["input"].(string) == url {
return msmtJSON, nil
}
}
@ -166,21 +159,18 @@ func GetMeasurementCounts(sess sqlbuilder.Database, resultID int64) (uint64, uin
func ListResults(sess sqlbuilder.Database) ([]ResultNetwork, []ResultNetwork, error) {
doneResults := []ResultNetwork{}
incompleteResults := []ResultNetwork{}
req := sess.Select(
db.Raw("networks.*"),
db.Raw("results.*"),
).From("results").
Join("networks").On("results.network_id = networks.network_id").
OrderBy("results.result_start_time")
if err := req.Where("result_is_done = true").All(&doneResults); err != nil {
return doneResults, incompleteResults, errors.Wrap(err, "failed to get result done list")
}
if err := req.Where("result_is_done = false").All(&incompleteResults); err != nil {
return doneResults, incompleteResults, errors.Wrap(err, "failed to get result done list")
}
return doneResults, incompleteResults, nil
}

View File

@ -1,7 +1,6 @@
package util
import (
"bufio"
"bytes"
"fmt"
"os"
@ -30,6 +29,8 @@ var ansiEscapes = regexp.MustCompile(`[\x1B\x9B][[\]()#;?]*` +
`(?:(?:(?:[a-zA-Z\d]*(?:;[a-zA-Z\\d]*)*)?\x07)` +
`|(?:(?:\d{1,4}(?:;\d{0,4})*)?[\dA-PRZcf-ntqry=><~]))`)
// EscapeAwareRuneCountInString counts the number of runes in a
// string taking into account escape sequences.
func EscapeAwareRuneCountInString(s string) int {
n := utf8.RuneCountInString(s)
for _, sm := range ansiEscapes.FindAllString(s, -1) {
@ -38,6 +39,7 @@ func EscapeAwareRuneCountInString(s string) int {
return n
}
// RightPadd adds right padding in from of a string
func RightPad(str string, length int) string {
c := length - EscapeAwareRuneCountInString(str)
if c < 0 {
@ -113,17 +115,3 @@ func WrapString(s string, lim uint) string {
return buf.String()
}
// ReadLine will read a single line from a bufio.Reader
func ReadLine(r *bufio.Reader) (string, error) {
var (
isPrefix bool
err error
line, ln []byte
)
for isPrefix && err == nil {
line, isPrefix, err = r.ReadLine()
ln = append(ln, line...)
}
return string(ln), err
}