From 77feba689deceb514f7e5d290b6e830db30a82c5 Mon Sep 17 00:00:00 2001 From: Travis Date: Sat, 29 Apr 2017 14:35:21 -0500 Subject: [PATCH 1/3] remove the support for `.` in Index and Frame names --- frame_test.go | 34 +++++++++++++++++++++++++++++++++ pilosa.go | 5 ++--- server/server_test.go | 44 +++++++++++++++++++++---------------------- 3 files changed, 58 insertions(+), 25 deletions(-) diff --git a/frame_test.go b/frame_test.go index a8bf3aceb..81d5bb770 100644 --- a/frame_test.go +++ b/frame_test.go @@ -80,6 +80,40 @@ func TestFrame_NameRestriction(t *testing.T) { } } +// Ensure that frame name validation is consistent. +func TestFrame_NameValidation(t *testing.T) { + validFrameNames := []string{ + "foo", + "hyphen-ated", + "under_score", + "abc123", + "trailing_", + } + invalidFrameNames := []string{ + "", + "x.y", + "_foo", + "-bar", + "abc def", + "camelCase", + "UPPERCASE", + "a12345678901234567890123456789012345678901234567890123456789012345", + } + + for _, name := range validFrameNames { + _, err := pilosa.NewFrame("", "i", name) + if err != nil { + t.Fatalf("unexpected frame name: %s %s", name, err) + } + } + for _, name := range invalidFrameNames { + _, err := pilosa.NewFrame("", "i", name) + if err == nil { + t.Fatalf("expected error on frame name: %s", name) + } + } +} + // Frame represents a test wrapper for pilosa.Frame. type Frame struct { *pilosa.Frame diff --git a/pilosa.go b/pilosa.go index 2092d1fe4..999627595 100644 --- a/pilosa.go +++ b/pilosa.go @@ -45,9 +45,8 @@ var ( ErrQueryRequired = errors.New("query required") ) -// Regular expression to valuate index and frame's name -// Todo: remove . when frame doesn't require . for topN -var nameRegexp = regexp.MustCompile(`^[a-z0-9][a-z0-9._-]{0,64}$`) +// Regular expression to validate index and frame names. +var nameRegexp = regexp.MustCompile(`^[a-z0-9][a-z0-9_-]{0,64}$`) // ColumnAttrSet represents a set of attributes for a vertical column in an index. // Can have a set of attributes attached to it. diff --git a/server/server_test.go b/server/server_test.go index 3a625ea3e..d981a4cf3 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -128,7 +128,7 @@ func TestMain_SetRowAttrs(t *testing.T) { client := m.Client() if err := client.CreateIndex(context.Background(), "i", pilosa.IndexOptions{}); err != nil && err != pilosa.ErrIndexExists { t.Fatal(err) - } else if err := client.CreateFrame(context.Background(), "i", "x.n", pilosa.FrameOptions{}); err != nil { + } else if err := client.CreateFrame(context.Background(), "i", "x", pilosa.FrameOptions{}); err != nil { t.Fatal(err) } else if err := client.CreateFrame(context.Background(), "i", "z", pilosa.FrameOptions{}); err != nil { t.Fatal(err) @@ -137,9 +137,9 @@ func TestMain_SetRowAttrs(t *testing.T) { } // Set bits on different rows in different frames. - if _, err := m.Query("i", "", `SetBit(rowID=1, frame="x.n", columnID=100)`); err != nil { + if _, err := m.Query("i", "", `SetBit(rowID=1, frame="x", columnID=100)`); err != nil { t.Fatal(err) - } else if _, err := m.Query("i", "", `SetBit(rowID=2, frame="x.n", columnID=100)`); err != nil { + } else if _, err := m.Query("i", "", `SetBit(rowID=2, frame="x", columnID=100)`); err != nil { t.Fatal(err) } else if _, err := m.Query("i", "", `SetBit(rowID=2, frame="z", columnID=100)`); err != nil { t.Fatal(err) @@ -148,9 +148,9 @@ func TestMain_SetRowAttrs(t *testing.T) { } // Set row attributes. - if _, err := m.Query("i", "", `SetRowAttrs(rowID=1, frame="x.n", x=100)`); err != nil { + if _, err := m.Query("i", "", `SetRowAttrs(rowID=1, frame="x", x=100)`); err != nil { t.Fatal(err) - } else if _, err := m.Query("i", "", `SetRowAttrs(rowID=2, frame="x.n", x=-200)`); err != nil { + } else if _, err := m.Query("i", "", `SetRowAttrs(rowID=2, frame="x", x=-200)`); err != nil { t.Fatal(err) } else if _, err := m.Query("i", "", `SetRowAttrs(rowID=2, frame="z", x=300)`); err != nil { t.Fatal(err) @@ -158,15 +158,15 @@ func TestMain_SetRowAttrs(t *testing.T) { t.Fatal(err) } - // Query row x.n/1. - if res, err := m.Query("i", "", `Bitmap(rowID=1, frame="x.n")`); err != nil { + // Query row x/1. + if res, err := m.Query("i", "", `Bitmap(rowID=1, frame="x")`); err != nil { t.Fatal(err) } else if res != `{"results":[{"attrs":{"x":100},"bits":[100]}]}`+"\n" { t.Fatalf("unexpected result: %s", res) } - // Query row x.n/2. - if res, err := m.Query("i", "", `Bitmap(rowID=2, frame="x.n")`); err != nil { + // Query row x/2. + if res, err := m.Query("i", "", `Bitmap(rowID=2, frame="x")`); err != nil { t.Fatal(err) } else if res != `{"results":[{"attrs":{"x":-200},"bits":[100]}]}`+"\n" { t.Fatalf("unexpected result: %s", res) @@ -177,7 +177,7 @@ func TestMain_SetRowAttrs(t *testing.T) { } // Query rows after reopening. - if res, err := m.Query("i", "columnAttrs=true", `Bitmap(rowID=1, frame="x.n")`); err != nil { + if res, err := m.Query("i", "columnAttrs=true", `Bitmap(rowID=1, frame="x")`); err != nil { t.Fatal(err) } else if res != `{"results":[{"attrs":{"x":100},"bits":[100]}]}`+"\n" { t.Fatalf("unexpected result(reopen): %s", res) @@ -188,8 +188,8 @@ func TestMain_SetRowAttrs(t *testing.T) { } else if res != `{"results":[{"attrs":{"x":-0.44},"bits":[100]}]}`+"\n" { t.Fatalf("unexpected result(reopen): %s", res) } - // Query row x.n/2. - if res, err := m.Query("i", "", `Bitmap(rowID=2, frame="x.n")`); err != nil { + // Query row x/2. + if res, err := m.Query("i", "", `Bitmap(rowID=2, frame="x")`); err != nil { t.Fatal(err) } else if res != `{"results":[{"attrs":{"x":-200},"bits":[100]}]}`+"\n" { t.Fatalf("unexpected result: %s", res) @@ -205,14 +205,14 @@ func TestMain_SetColumnAttrs(t *testing.T) { client := m.Client() if err := client.CreateIndex(context.Background(), "i", pilosa.IndexOptions{}); err != nil && err != pilosa.ErrIndexExists { t.Fatal(err) - } else if err := client.CreateFrame(context.Background(), "i", "x.n", pilosa.FrameOptions{}); err != nil { + } else if err := client.CreateFrame(context.Background(), "i", "x", pilosa.FrameOptions{}); err != nil { t.Fatal(err) } // Set bits on row. - if _, err := m.Query("i", "", `SetBit(rowID=1, frame="x.n", columnID=100)`); err != nil { + if _, err := m.Query("i", "", `SetBit(rowID=1, frame="x", columnID=100)`); err != nil { t.Fatal(err) - } else if _, err := m.Query("i", "", `SetBit(rowID=1, frame="x.n", columnID=101)`); err != nil { + } else if _, err := m.Query("i", "", `SetBit(rowID=1, frame="x", columnID=101)`); err != nil { t.Fatal(err) } @@ -222,7 +222,7 @@ func TestMain_SetColumnAttrs(t *testing.T) { } // Query row. - if res, err := m.Query("i", "columnAttrs=true", `Bitmap(rowID=1, frame="x.n")`); err != nil { + if res, err := m.Query("i", "columnAttrs=true", `Bitmap(rowID=1, frame="x")`); err != nil { t.Fatal(err) } else if res != `{"results":[{"attrs":{},"bits":[100,101]}],"columnAttrs":[{"id":100,"attrs":{"foo":"bar"}}]}`+"\n" { t.Fatalf("unexpected result: %s", res) @@ -233,7 +233,7 @@ func TestMain_SetColumnAttrs(t *testing.T) { } // Query row after reopening. - if res, err := m.Query("i", "columnAttrs=true", `Bitmap(rowID=1, frame="x.n")`); err != nil { + if res, err := m.Query("i", "columnAttrs=true", `Bitmap(rowID=1, frame="x")`); err != nil { t.Fatal(err) } else if res != `{"results":[{"attrs":{},"bits":[100,101]}],"columnAttrs":[{"id":100,"attrs":{"foo":"bar"}}]}`+"\n" { t.Fatalf("unexpected result(reopen): %s", res) @@ -249,14 +249,14 @@ func TestMain_SetColumnAttrsWithColumnOption(t *testing.T) { client := m.Client() if err := client.CreateIndex(context.Background(), "i", pilosa.IndexOptions{ColumnLabel: "col"}); err != nil && err != pilosa.ErrIndexExists { t.Fatal(err) - } else if err := client.CreateFrame(context.Background(), "i", "x.n", pilosa.FrameOptions{}); err != nil { + } else if err := client.CreateFrame(context.Background(), "i", "x", pilosa.FrameOptions{}); err != nil { t.Fatal(err) } // Set bits on row. - if _, err := m.Query("i", "", `SetBit(rowID=1, frame="x.n", col=100)`); err != nil { + if _, err := m.Query("i", "", `SetBit(rowID=1, frame="x", col=100)`); err != nil { t.Fatal(err) - } else if _, err := m.Query("i", "", `SetBit(rowID=1, frame="x.n", col=101)`); err != nil { + } else if _, err := m.Query("i", "", `SetBit(rowID=1, frame="x", col=101)`); err != nil { t.Fatal(err) } @@ -266,7 +266,7 @@ func TestMain_SetColumnAttrsWithColumnOption(t *testing.T) { } // Query row. - if res, err := m.Query("i", "columnAttrs=true", `Bitmap(rowID=1, frame="x.n")`); err != nil { + if res, err := m.Query("i", "columnAttrs=true", `Bitmap(rowID=1, frame="x")`); err != nil { t.Fatal(err) } else if res != `{"results":[{"attrs":{},"bits":[100,101]}],"columnAttrs":[{"id":100,"attrs":{"foo":"bar"}}]}`+"\n" { t.Fatalf("unexpected result: %s", res) @@ -652,7 +652,7 @@ func GenerateSetCommands(n int, rand *rand.Rand) []SetCommand { for i := range cmds { cmds[i] = SetCommand{ ID: uint64(rand.Intn(1000)), - Frame: "x.n", + Frame: "x", ColumnID: uint64(rand.Intn(10)), } } From 692d997cab828f780ae64071d6e59602fa36520b Mon Sep 17 00:00:00 2001 From: Travis Date: Sat, 29 Apr 2017 15:01:18 -0500 Subject: [PATCH 2/3] add `labelRegexp` to validate row and column labels --- frame.go | 2 +- frame_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-- index.go | 2 +- pilosa.go | 17 ++++++++++++++--- 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/frame.go b/frame.go index 5e83e2984..1fd470258 100644 --- a/frame.go +++ b/frame.go @@ -140,7 +140,7 @@ func (f *Frame) SetRowLabel(v string) error { } // Make sure rowLabel is valid name - err := ValidateName(v) + err := ValidateLabel(v) if err != nil { return err } diff --git a/frame_test.go b/frame_test.go index 81d5bb770..a6001e8d2 100644 --- a/frame_test.go +++ b/frame_test.go @@ -100,20 +100,66 @@ func TestFrame_NameValidation(t *testing.T) { "a12345678901234567890123456789012345678901234567890123456789012345", } + path, err := ioutil.TempDir("", "pilosa-frame-") + if err != nil { + panic(err) + } for _, name := range validFrameNames { - _, err := pilosa.NewFrame("", "i", name) + _, err := pilosa.NewFrame(path, "i", name) if err != nil { t.Fatalf("unexpected frame name: %s %s", name, err) } } for _, name := range invalidFrameNames { - _, err := pilosa.NewFrame("", "i", name) + _, err := pilosa.NewFrame(path, "i", name) if err == nil { t.Fatalf("expected error on frame name: %s", name) } } } +// Ensure that frame RowLable validation is consistent. +func TestFrame_RowLabelValidation(t *testing.T) { + validRowLabels := []string{ + "", + "foo", + "hyphen-ated", + "under_score", + "abc123", + "trailing_", + "camelCase", + "UPPERCASE", + } + invalidRowLabels := []string{ + "x.y", + "_foo", + "-bar", + "abc def", + "a12345678901234567890123456789012345678901234567890123456789012345", + } + + path, err := ioutil.TempDir("", "pilosa-frame-") + if err != nil { + panic(err) + } + f, err := pilosa.NewFrame(path, "i", "f") + if err != nil { + t.Fatalf("unexpected frame error: %s", err) + } + + for _, label := range validRowLabels { + if err := f.SetRowLabel(label); err != nil { + t.Fatalf("unexpected row label: %s %s", label, err) + } + } + for _, label := range invalidRowLabels { + if err := f.SetRowLabel(label); err == nil { + t.Fatalf("expected error on row label: %s", label) + } + } + +} + // Frame represents a test wrapper for pilosa.Frame. type Frame struct { *pilosa.Frame diff --git a/index.go b/index.go index 1be1363d0..b7c516b75 100644 --- a/index.go +++ b/index.go @@ -108,7 +108,7 @@ func (i *Index) SetColumnLabel(v string) error { } // Make sure columnLabel is valid name - err := ValidateName(v) + err := ValidateLabel(v) if err != nil { return err } diff --git a/pilosa.go b/pilosa.go index 999627595..6271bd8fa 100644 --- a/pilosa.go +++ b/pilosa.go @@ -38,7 +38,8 @@ var ( ErrInvalidView = errors.New("invalid view") ErrInvalidCacheType = errors.New("invalid cache type") - ErrName = errors.New("invalid index or frame's name, must match [a-z0-9_-]") + ErrName = errors.New("invalid index or frame's name, must match [a-z0-9_-]") + ErrLabel = errors.New("invalid row or column label, must match [A-Za-z0-9_-]") // ErrFragmentNotFound is returned when a fragment does not exist. ErrFragmentNotFound = errors.New("fragment not found") @@ -48,6 +49,9 @@ var ( // Regular expression to validate index and frame names. var nameRegexp = regexp.MustCompile(`^[a-z0-9][a-z0-9_-]{0,64}$`) +// Regular expression to validate row and column labels. +var labelRegexp = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_-]{0,64}$`) + // ColumnAttrSet represents a set of attributes for a vertical column in an index. // Can have a set of attributes attached to it. type ColumnAttrSet struct { @@ -103,9 +107,16 @@ const TimeFormat = "2006-01-02T15:04" // ValidateName ensures that the name is a valid format. func ValidateName(name string) error { - validName := nameRegexp.Match([]byte(name)) - if validName == false { + if nameRegexp.Match([]byte(name)) == false { return ErrName } return nil } + +// ValidateLabel ensures that the label is a valid format. +func ValidateLabel(label string) error { + if labelRegexp.Match([]byte(label)) == false { + return ErrLabel + } + return nil +} From b0fa067ffc583dd57e8ac5e8772b4a94ec461807 Mon Sep 17 00:00:00 2001 From: Travis Date: Sat, 29 Apr 2017 16:49:23 -0500 Subject: [PATCH 3/3] remove the ability to start an Index, Frame, ColumnLabel, or RowLabel with a number --- frame_test.go | 2 ++ pilosa.go | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/frame_test.go b/frame_test.go index a6001e8d2..fc5c01570 100644 --- a/frame_test.go +++ b/frame_test.go @@ -91,6 +91,7 @@ func TestFrame_NameValidation(t *testing.T) { } invalidFrameNames := []string{ "", + "123abc", "x.y", "_foo", "-bar", @@ -131,6 +132,7 @@ func TestFrame_RowLabelValidation(t *testing.T) { "UPPERCASE", } invalidRowLabels := []string{ + "123abc", "x.y", "_foo", "-bar", diff --git a/pilosa.go b/pilosa.go index 6271bd8fa..dd7402677 100644 --- a/pilosa.go +++ b/pilosa.go @@ -47,10 +47,10 @@ var ( ) // Regular expression to validate index and frame names. -var nameRegexp = regexp.MustCompile(`^[a-z0-9][a-z0-9_-]{0,64}$`) +var nameRegexp = regexp.MustCompile(`^[a-z][a-z0-9_-]{0,64}$`) // Regular expression to validate row and column labels. -var labelRegexp = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_-]{0,64}$`) +var labelRegexp = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_-]{0,64}$`) // ColumnAttrSet represents a set of attributes for a vertical column in an index. // Can have a set of attributes attached to it.