Merge pull request #358 from linhvo/retriction-frame

Restriction frame and db name

db.go

@@ -48,7 +48,12 @@ type DB struct {
 }
 
 // NewDB returns a new instance of DB.
-func NewDB(path, name string) *DB {
+func NewDB(path, name string) (*DB, error) {
+	err := ValidateName(name)
+	if err != nil {
+		return nil, err
+	}
+
 	return &DB{
 		path:           path,
 		name:           name,
@@ -61,7 +66,7 @@ func NewDB(path, name string) *DB {
 
 		stats:     NopStatsClient,
 		LogOutput: ioutil.Discard,
-	}
+	}, nil
 }
 
 // Name returns name of the database.
@@ -141,7 +146,10 @@ func (db *DB) openFrames() error {
 			continue
 		}
 
-		fr := db.newFrame(db.FramePath(filepath.Base(fi.Name())), filepath.Base(fi.Name()))
+		fr, err := db.newFrame(db.FramePath(filepath.Base(fi.Name())), filepath.Base(fi.Name()))
+		if err != nil {
+			return ErrName
+		}
 		if err := fr.Open(); err != nil {
 			return fmt.Errorf("open frame: name=%s, err=%s", fr.Name(), err)
 		}
@@ -312,12 +320,16 @@ func (db *DB) CreateFrameIfNotExists(name string, opt FrameOptions) (*Frame, err
 }
 
 func (db *DB) createFrame(name string, opt FrameOptions) (*Frame, error) {
+
 	if name == "" {
 		return nil, errors.New("frame name required")
 	}
 
 	// Initialize frame.
-	f := db.newFrame(db.FramePath(name), name)
+	f, err := db.newFrame(db.FramePath(name), name)
+	if err != nil {
+		return nil, err
+	}
 
 	// Open frame.
 	if err := f.Open(); err != nil {
@@ -335,11 +347,14 @@ func (db *DB) createFrame(name string, opt FrameOptions) (*Frame, error) {
 	return f, nil
 }
 
-func (db *DB) newFrame(path, name string) *Frame {
-	f := NewFrame(path, db.name, name)
+func (db *DB) newFrame(path, name string) (*Frame, error) {
+	f, err := NewFrame(path, db.name, name)
+	if err != nil {
+		return nil, err
+	}
 	f.LogOutput = db.LogOutput
 	f.stats = db.stats.WithTags(fmt.Sprintf("frame:%s", name))
-	return f
+	return f, nil
 }
 
 // DeleteFrame removes a frame from the database.

db_test.go

@@ -89,7 +89,11 @@ func NewDB() *DB {
 	if err != nil {
 		panic(err)
 	}
-	return &DB{DB: pilosa.NewDB(path, "d")}
+	db, err := pilosa.NewDB(path, "d")
+	if err != nil {
+		panic(err)
+	}
+	return &DB{DB: db}
 }
 
 // MustOpenDB returns a new, opened database at a temporary path. Panic on error.
@@ -109,12 +113,16 @@ func (db *DB) Close() error {
 
 // Reopen closes the database and reopens it.
 func (db *DB) Reopen() error {
+	var err error
 	if err := db.DB.Close(); err != nil {
 		return err
 	}
 
 	path, name := db.Path(), db.Name()
-	db.DB = pilosa.NewDB(path, name)
+	db.DB, err = pilosa.NewDB(path, name)
+	if err != nil {
+		return err
+	}
 
 	if err := db.Open(); err != nil {
 		return err
@@ -130,3 +138,15 @@ func (db *DB) MustSetBit(name string, bitmapID, profileID uint64, t *time.Time)
 	}
 	return changed
 }
+
+// Ensure database can delete a frame.
+func TestDB_InvalidName(t *testing.T) {
+	path, err := ioutil.TempDir("", "pilosa-db-")
+	if err != nil {
+		panic(err)
+	}
+	db, err := pilosa.NewDB(path, "ABC")
+	if db != nil {
+		t.Fatalf("unexpected db name %s", db)
+	}
+}

executor_test.go

@@ -182,7 +182,7 @@ func TestExecutor_Execute_SetBitmapAttrs(t *testing.T) {
 	db := idx.MustCreateDBIfNotExists("d", pilosa.DBOptions{})
 	if _, err := db.CreateFrameIfNotExists("f", pilosa.FrameOptions{}); err != nil {
 		t.Fatal(err)
-	} else if _, err := db.CreateFrameIfNotExists("XXX", pilosa.FrameOptions{}); err != nil {
+	} else if _, err := db.CreateFrameIfNotExists("xxx", pilosa.FrameOptions{}); err != nil {
 		t.Fatal(err)
 	}
 
@@ -195,7 +195,7 @@ func TestExecutor_Execute_SetBitmapAttrs(t *testing.T) {
 	if _, err := e.Execute(context.Background(), "d", MustParse(`SetBitmapAttrs(id=200, frame=f, YYY=1)`), nil, nil); err != nil {
 		t.Fatal(err)
 	}
-	if _, err := e.Execute(context.Background(), "d", MustParse(`SetBitmapAttrs(id=10, frame=XXX, YYY=1)`), nil, nil); err != nil {
+	if _, err := e.Execute(context.Background(), "d", MustParse(`SetBitmapAttrs(id=10, frame=xxx, YYY=1)`), nil, nil); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := e.Execute(context.Background(), "d", MustParse(`SetBitmapAttrs(id=10, frame=f, baz=123, bat=true)`), nil, nil); err != nil {

frame.go

@@ -46,7 +46,12 @@ type Frame struct {
 }
 
 // NewFrame returns a new instance of frame.
-func NewFrame(path, db, name string) *Frame {
+func NewFrame(path, db, name string) (*Frame, error) {
+	err := ValidateName(name)
+	if err != nil {
+		return nil, err
+	}
+
 	return &Frame{
 		path: path,
 		db:   db,
@@ -60,7 +65,7 @@ func NewFrame(path, db, name string) *Frame {
 		rowLabel: DefaultRowLabel,
 
 		LogOutput: ioutil.Discard,
-	}
+	}, nil
 }
 
 // Name returns the name the frame was initialized with.

frame_test.go

@@ -65,8 +65,11 @@ func NewFrame() *Frame {
 	if err != nil {
 		panic(err)
 	}
-
-	return &Frame{Frame: pilosa.NewFrame(path, "d", "f")}
+	frame, err := pilosa.NewFrame(path, "d", "f")
+	if err != nil {
+		panic(err)
+	}
+	return &Frame{Frame: frame}
 }
 
 // MustOpenFrame returns a new, opened frame at a temporary path. Panic on error.
@@ -86,15 +89,31 @@ func (f *Frame) Close() error {
 
 // Reopen closes the database and reopens it.
 func (f *Frame) Reopen() error {
+	var err error
 	if err := f.Frame.Close(); err != nil {
 		return err
 	}
 
 	path, db, name := f.Path(), f.DB(), f.Name()
-	f.Frame = pilosa.NewFrame(path, db, name)
+	f.Frame, err = pilosa.NewFrame(path, db, name)
+	if err != nil {
+		return err
+	}
 
 	if err := f.Open(); err != nil {
 		return err
 	}
 	return nil
 }
+
+// NewFrame returns a new instance of Frame d/0.
+func TestFrame_NameRestriction(t *testing.T) {
+	path, err := ioutil.TempDir("", "pilosa-frame-")
+	if err != nil {
+		panic(err)
+	}
+	frame, err := pilosa.NewFrame(path, "d", "ABC")
+	if frame != nil {
+		t.Fatalf("unexpected frame name %s", err)
+	}
+}

index.go

@@ -78,7 +78,10 @@ func (i *Index) Open() error {
 
 		i.logger().Printf("opening database: %s", filepath.Base(fi.Name()))
 
-		db := i.newDB(i.DBPath(filepath.Base(fi.Name())), filepath.Base(fi.Name()))
+		db, err := i.newDB(i.DBPath(filepath.Base(fi.Name())), filepath.Base(fi.Name()))
+		if err != nil {
+			return ErrName
+		}
 		if err := db.Open(); err != nil {
 			return fmt.Errorf("open db: name=%s, err=%s", db.Name(), err)
 		}
@@ -193,7 +196,11 @@ func (i *Index) createDB(name string, opt DBOptions) (*DB, error) {
 	}
 
 	// Otherwise create a new database.
-	db := i.newDB(i.DBPath(name), name)
+	db, err := i.newDB(i.DBPath(name), name)
+	if err != nil {
+		return nil, err
+	}
+
 	if err := db.Open(); err != nil {
 		return nil, err
 	}
@@ -208,11 +215,14 @@ func (i *Index) createDB(name string, opt DBOptions) (*DB, error) {
 	return db, nil
 }
 
-func (i *Index) newDB(path, name string) *DB {
-	db := NewDB(path, name)
+func (i *Index) newDB(path, name string) (*DB, error) {
+	db, err := NewDB(path, name)
+	if err != nil {
+		return nil, err
+	}
 	db.LogOutput = i.LogOutput
 	db.stats = i.Stats.WithTags(fmt.Sprintf("db:%s", db.Name()))
-	return db
+	return db, nil
 }
 
 // DeleteDB removes a database from the index.

pilosa.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 
 	"github.com/pilosa/pilosa/internal"
+	"regexp"
 )
 
 // System errors.
@@ -18,10 +19,18 @@ var (
 	ErrFrameExists   = errors.New("frame already exists")
 	ErrFrameNotFound = errors.New("frame not found")
 
+	// ErrFrameRequired is returned when no frame is specified.
+	ErrName = errors.New("name restricted to [a-z0-9_-]")
+
+	// ErrFragmentNotFound is returned when a fragment does not exist.
 	ErrFragmentNotFound = errors.New("fragment not found")
 	ErrQueryRequired    = errors.New("query required")
 )
 
+// Regular expression to valuate db and frame's name
+// Todo: remove . when frame doesn't require . for topN
+var nameRegexp = regexp.MustCompile(`^([a-z0-9._-]{1,64}$)`)
+
 // Profile represents vertical column in a database.
 // A profile can have a set of attributes attached to it.
 type Profile struct {
@@ -74,3 +83,13 @@ func decodeProfile(pb *internal.Profile) *Profile {
 
 // TimeFormat is the go-style time format used to parse string dates.
 const TimeFormat = "2006-01-02T15:04"
+
+
+// Restrict name using regex
+func ValidateName(name string) error {
+	validName := nameRegexp.Match([]byte(name))
+	if validName == false{
+		return ErrName
+	}
+	return nil
+}

pilosactl/import.go

@@ -104,7 +104,6 @@ func (cmd *ImportCommand) Run(ctx context.Context) error {
 	} else if len(cmd.Paths) == 0 {
 		return errors.New("path required")
 	}
-
 	// Create a client to the server.
 	client, err := pilosa.NewClient(cmd.Host)
 	if err != nil {