From f06c5320478b7c5c1a88e057129bbc3e17381534 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 19 Jun 2018 08:21:10 -0500 Subject: [PATCH 01/16] remove a few unecessary lines from SetupNetworking NewServer calls LoadNodeID, and NopBroadcaster and NopBroadcastReceiver are already set up as the defaults. --- server/server.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/server/server.go b/server/server.go index b6bf6e864..b9493e8ca 100644 --- a/server/server.go +++ b/server/server.go @@ -249,9 +249,6 @@ func (m *Command) SetupServer() error { // SetupNetworking sets up internode communication based on the configuration. func (m *Command) SetupNetworking() error { - - m.Server.NodeID = m.Server.LoadNodeID() - if m.Config.Cluster.Disabled { m.Server.Cluster.Static = true m.Server.Cluster.Coordinator = m.Server.NodeID @@ -265,9 +262,7 @@ func (m *Command) SetupNetworking() error { }) } - m.Server.Broadcaster = pilosa.NopBroadcaster m.Server.Cluster.MemberSet = pilosa.NewStaticMemberSet(m.Server.Cluster.Nodes) - m.Server.BroadcastReceiver = pilosa.NopBroadcastReceiver return nil } From 719241f0d9572911838805e16d68893e6b1ff8c0 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 19 Jun 2018 08:31:17 -0500 Subject: [PATCH 02/16] move some silly comments around --- server.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/server.go b/server.go index dd26a2c0b..39074de95 100644 --- a/server.go +++ b/server.go @@ -263,6 +263,7 @@ func NewServer(opts ...ServerOption) (*Server, error) { s.URI.SetPort(uint16(s.ln.Addr().(*net.TCPAddr).Port)) } + // Get or create NodeID. s.NodeID = s.LoadNodeID() // Set Cluster Node. node := &Node{ @@ -271,6 +272,8 @@ func NewServer(opts ...ServerOption) (*Server, error) { IsCoordinator: s.Cluster.Coordinator == s.NodeID, } s.Cluster.Node = node + + // Append the NodeID tag to stats. s.Holder.Stats = s.Holder.Stats.WithTags(fmt.Sprintf("NodeID:%s", s.NodeID)) s.executor.Holder = s.Holder @@ -298,14 +301,6 @@ func (s *Server) Open() error { log.Println(errors.Wrap(err, "logging startup")) } - // Get or create NodeID. - - // Append the NodeID tag to stats. - - // Create default HTTP client - - // Create executor for executing queries. - // Cluster settings. s.Cluster.Broadcaster = s.Broadcaster s.Cluster.MaxWritesPerRequest = s.maxWritesPerRequest From 33b3a14b2464cbc2245b6002fd7c8a98115b9445 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 19 Jun 2018 16:28:54 -0500 Subject: [PATCH 03/16] move static cluster setup logic into Server/Cluster --- cluster.go | 17 +++++++++++++++++ server.go | 19 +++++++++++++++++-- server/server.go | 18 +++++------------- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/cluster.go b/cluster.go index 8dc3a60d1..7cea731bd 100644 --- a/cluster.go +++ b/cluster.go @@ -1801,3 +1801,20 @@ func (c *Cluster) mergeClusterStatus(cs *internal.ClusterStatus) error { return nil } + +func (c *Cluster) setStatic(hosts []string) error { + if len(hosts) == 0 { + return errors.New("must specify at least one host") + } + c.Static = true + c.Coordinator = c.Node.ID + for _, address := range hosts { + uri, err := NewURIFromAddress(address) + if err != nil { + return errors.Wrap(err, "getting URI") + } + c.Nodes = append(c.Nodes, &Node{URI: *uri}) + } + c.MemberSet = NewStaticMemberSet(c.Nodes) + return nil +} diff --git a/server.go b/server.go index 39074de95..9e1b282b8 100644 --- a/server.go +++ b/server.go @@ -57,6 +57,7 @@ type Server struct { TranslateFile *TranslateFile diagnostics *DiagnosticsCollector executor *Executor + hosts []string // External handler Handler @@ -207,6 +208,15 @@ func OptServerURI(uri *URI) ServerOption { } } +// OptClusterStatic tells the server to use a static cluster with the defined +// hosts. Mostly used for testing. +func OptServerClusterStatic(hosts []string) ServerOption { + return func(s *Server) error { + s.hosts = hosts + return nil + } +} + // NewServer returns a new instance of Server. func NewServer(opts ...ServerOption) (*Server, error) { s := &Server{ @@ -272,6 +282,12 @@ func NewServer(opts ...ServerOption) (*Server, error) { IsCoordinator: s.Cluster.Coordinator == s.NodeID, } s.Cluster.Node = node + if len(s.hosts) > 0 { + err := s.Cluster.setStatic(s.hosts) + if err != nil { + return nil, errors.Wrap(err, "setting cluster static") + } + } // Append the NodeID tag to stats. s.Holder.Stats = s.Holder.Stats.WithTags(fmt.Sprintf("NodeID:%s", s.NodeID)) @@ -290,9 +306,8 @@ func NewServer(opts ...ServerOption) (*Server, error) { // Open opens and initializes the server. func (s *Server) Open() error { s.logger.Printf("open server") - // s.ln can be configured prior to Open() via s.OpenListener(). if s.ln == nil { - return errors.New("Must pass a listener option to NewServer") + return errors.New("must pass a listener option to NewServer") } // Log startup diff --git a/server/server.go b/server/server.go index b9493e8ca..373975e5f 100644 --- a/server/server.go +++ b/server/server.go @@ -209,6 +209,10 @@ func (m *Command) SetupServer() error { if err != nil { return errors.Wrap(err, "new stats client") } + var hosts []string + if m.Config.Cluster.Disabled { + hosts = m.Config.Cluster.Hosts + } ln, err := getListener(*uri, TLSConfig) if err != nil { @@ -242,6 +246,7 @@ func (m *Command) SetupServer() error { pilosa.OptServerURI(uri), pilosa.OptServerInternalClient(http.NewInternalClientFromURI(uri, c)), pilosa.OptServerPrimaryTranslateStore(primaryTranslateStore), + pilosa.OptServerClusterStatic(hosts), ) return errors.Wrap(err, "new server") @@ -250,19 +255,6 @@ func (m *Command) SetupServer() error { // SetupNetworking sets up internode communication based on the configuration. func (m *Command) SetupNetworking() error { if m.Config.Cluster.Disabled { - m.Server.Cluster.Static = true - m.Server.Cluster.Coordinator = m.Server.NodeID - for _, address := range m.Config.Cluster.Hosts { - uri, err := pilosa.NewURIFromAddress(address) - if err != nil { - return errors.Wrap(err, "getting URI") - } - m.Server.Cluster.Nodes = append(m.Server.Cluster.Nodes, &pilosa.Node{ - URI: *uri, - }) - } - - m.Server.Cluster.MemberSet = pilosa.NewStaticMemberSet(m.Server.Cluster.Nodes) return nil } From 69b1f2ea97bdb072bf64a827a4b703b40b65d334 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 19 Jun 2018 17:02:44 -0500 Subject: [PATCH 04/16] make behavior equivalent to pre-change to stop test from failing --- cluster.go | 3 --- server.go | 22 ++++++++++++---------- server/server.go | 6 +----- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/cluster.go b/cluster.go index 7cea731bd..c8cec60a2 100644 --- a/cluster.go +++ b/cluster.go @@ -1803,9 +1803,6 @@ func (c *Cluster) mergeClusterStatus(cs *internal.ClusterStatus) error { } func (c *Cluster) setStatic(hosts []string) error { - if len(hosts) == 0 { - return errors.New("must specify at least one host") - } c.Static = true c.Coordinator = c.Node.ID for _, address := range hosts { diff --git a/server.go b/server.go index 9e1b282b8..14d9bb2b6 100644 --- a/server.go +++ b/server.go @@ -52,12 +52,13 @@ type Server struct { closing chan struct{} // Internal - Holder *Holder - Cluster *Cluster - TranslateFile *TranslateFile - diagnostics *DiagnosticsCollector - executor *Executor - hosts []string + Holder *Holder + Cluster *Cluster + TranslateFile *TranslateFile + diagnostics *DiagnosticsCollector + executor *Executor + hosts []string + clusterDisabled bool // External handler Handler @@ -208,11 +209,12 @@ func OptServerURI(uri *URI) ServerOption { } } -// OptClusterStatic tells the server to use a static cluster with the defined -// hosts. Mostly used for testing. -func OptServerClusterStatic(hosts []string) ServerOption { +// OptClusterDisabled tells the server whether to use a static cluster with the +// defined hosts. Mostly used for testing. +func OptServerClusterDisabled(disabled bool, hosts []string) ServerOption { return func(s *Server) error { s.hosts = hosts + s.clusterDisabled = disabled return nil } } @@ -282,7 +284,7 @@ func NewServer(opts ...ServerOption) (*Server, error) { IsCoordinator: s.Cluster.Coordinator == s.NodeID, } s.Cluster.Node = node - if len(s.hosts) > 0 { + if s.clusterDisabled { err := s.Cluster.setStatic(s.hosts) if err != nil { return nil, errors.Wrap(err, "setting cluster static") diff --git a/server/server.go b/server/server.go index 373975e5f..4898088f3 100644 --- a/server/server.go +++ b/server/server.go @@ -209,10 +209,6 @@ func (m *Command) SetupServer() error { if err != nil { return errors.Wrap(err, "new stats client") } - var hosts []string - if m.Config.Cluster.Disabled { - hosts = m.Config.Cluster.Hosts - } ln, err := getListener(*uri, TLSConfig) if err != nil { @@ -246,7 +242,7 @@ func (m *Command) SetupServer() error { pilosa.OptServerURI(uri), pilosa.OptServerInternalClient(http.NewInternalClientFromURI(uri, c)), pilosa.OptServerPrimaryTranslateStore(primaryTranslateStore), - pilosa.OptServerClusterStatic(hosts), + pilosa.OptServerClusterDisabled(m.Config.Cluster.Disabled, m.Config.Cluster.Hosts), ) return errors.Wrap(err, "new server") From ca6b3b55244b5721419efc51673bdd2507c7269c Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Tue, 19 Jun 2018 17:48:37 -0500 Subject: [PATCH 05/16] simplify test cluter addNode signature --- cluster_internal_test.go | 20 ++++++++++---------- utils_internal_test.go | 6 +++--- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/cluster_internal_test.go b/cluster_internal_test.go index 8840c9dbb..c6c770232 100644 --- a/cluster_internal_test.go +++ b/cluster_internal_test.go @@ -552,7 +552,7 @@ func TestCluster_ResizeStates(t *testing.T) { t.Run("Single node, in topology", func(t *testing.T) { tc := NewClusterCluster(0) - tc.addNode(false) + tc.addNode() node := tc.Clusters[0] @@ -580,7 +580,7 @@ func TestCluster_ResizeStates(t *testing.T) { t.Run("Single node, not in topology", func(t *testing.T) { tc := NewClusterCluster(0) - tc.addNode(false) + tc.addNode() node := tc.Clusters[0] @@ -605,14 +605,14 @@ func TestCluster_ResizeStates(t *testing.T) { t.Run("Multiple nodes, no data", func(t *testing.T) { tc := NewClusterCluster(0) - tc.addNode(false) + tc.addNode() // Open TestCluster. if err := tc.Open(); err != nil { t.Fatal(err) } - tc.addNode(false) + tc.addNode() node0 := tc.Clusters[0] node1 := tc.Clusters[1] @@ -643,7 +643,7 @@ func TestCluster_ResizeStates(t *testing.T) { t.Run("Multiple nodes, in/not in topology", func(t *testing.T) { tc := NewClusterCluster(0) - tc.addNode(false) + tc.addNode() node0 := tc.Clusters[0] // write topology to data file @@ -664,12 +664,12 @@ func TestCluster_ResizeStates(t *testing.T) { // Expect an error by adding a node not in the topology. expectedError := "host is not in topology: node1" - err := tc.addNode(false) + err := tc.addNode() if err == nil || err.Error() != expectedError { t.Errorf("did not receive expected error: %s", expectedError) } - tc.addNode(false) + tc.addNode() node2 := tc.Clusters[2] // Ensure that node comes up in state NORMAL. @@ -687,7 +687,7 @@ func TestCluster_ResizeStates(t *testing.T) { t.Run("Multiple nodes, with data", func(t *testing.T) { tc := NewClusterCluster(0) - tc.addNode(false) + tc.addNode() node0 := tc.Clusters[0] // Open TestCluster. @@ -709,8 +709,8 @@ func TestCluster_ResizeStates(t *testing.T) { node0Fragment := node0View.Fragment(1) node0Checksum := node0Fragment.Checksum() - // AddNode needs to block until the resize process has completed. - tc.addNode(false) + // addNode needs to block until the resize process has completed. + tc.addNode() node1 := tc.Clusters[1] // Ensure that nodes come up in state NORMAL. diff --git a/utils_internal_test.go b/utils_internal_test.go index d1b49db03..f120019b9 100644 --- a/utils_internal_test.go +++ b/utils_internal_test.go @@ -150,11 +150,11 @@ func (t *ClusterCluster) clusterByID(id string) *Cluster { return nil } -// AddNode adds a node to the cluster and (potentially) starts a resize job. -func (t *ClusterCluster) addNode(saveTopology bool) error { +// addNode adds a node to the cluster and (potentially) starts a resize job. +func (t *ClusterCluster) addNode() error { id := len(t.Clusters) - c, err := t.addCluster(id, saveTopology) + c, err := t.addCluster(id, false) if err != nil { return err } From 8c35cb89bb1438ca0695fc0a1e3dd9037153033c Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Tue, 19 Jun 2018 17:54:16 -0500 Subject: [PATCH 06/16] rename test/frame.go to test/field.go --- test/{frame.go => field.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{frame.go => field.go} (100%) diff --git a/test/frame.go b/test/field.go similarity index 100% rename from test/frame.go rename to test/field.go From c77b7d5ca531436c27fb62da2da2fe1b0486c0e3 Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Tue, 19 Jun 2018 18:15:38 -0500 Subject: [PATCH 07/16] remove view argument from Field.SetBit and Field.ClearBit --- cluster_internal_test.go | 12 ++++++------ executor.go | 16 ++++++++-------- field.go | 22 ++++++++-------------- holder_test.go | 6 +++--- http/handler_test.go | 8 ++++---- test/field.go | 10 ---------- test/holder.go | 4 ++-- utils_internal_test.go | 4 ++-- 8 files changed, 33 insertions(+), 49 deletions(-) diff --git a/cluster_internal_test.go b/cluster_internal_test.go index 8840c9dbb..39742217e 100644 --- a/cluster_internal_test.go +++ b/cluster_internal_test.go @@ -153,19 +153,19 @@ func TestFragSources(t *testing.T) { if err != nil { t.Fatal(err) } - _, err = field.SetBit("standard", 1, 101, nil) + _, err = field.SetBit(1, 101, nil) if err != nil { t.Fatal(err) } - _, err = field.SetBit("standard", 1, 1300000, nil) + _, err = field.SetBit(1, 1300000, nil) if err != nil { t.Fatal(err) } - _, err = field.SetBit("standard", 1, 2600000, nil) + _, err = field.SetBit(1, 2600000, nil) if err != nil { t.Fatal(err) } - _, err = field.SetBit("standard", 1, 3900000, nil) + _, err = field.SetBit(1, 3900000, nil) if err != nil { t.Fatal(err) } @@ -699,8 +699,8 @@ func TestCluster_ResizeStates(t *testing.T) { if err := tc.CreateField("i", "f", FieldOptions{}); err != nil { t.Fatal(err) } - tc.SetBit("i", "f", "standard", 1, 101, nil) - tc.SetBit("i", "f", "standard", 1, 1300000, nil) + tc.SetBit("i", "f", 1, 101, nil) + tc.SetBit("i", "f", 1, 1300000, nil) // Before starting the resize, get the CheckSum to use for // comparison later. diff --git a/executor.go b/executor.go index a99753023..c4abe0bd0 100644 --- a/executor.go +++ b/executor.go @@ -1026,17 +1026,17 @@ func (e *Executor) executeClearBit(ctx context.Context, index string, c *pql.Cal return false, fmt.Errorf("ClearBit col field '%v' required", columnLabel) } - return e.executeClearBitView(ctx, index, c, f, ViewStandard, colID, rowID, opt) + return e.executeClearBitField(ctx, index, c, f, colID, rowID, opt) } -// executeClearBitView executes a ClearBit() call for a single view. -func (e *Executor) executeClearBitView(ctx context.Context, index string, c *pql.Call, f *Field, view string, colID, rowID uint64, opt *ExecOptions) (bool, error) { +// executeClearBitField executes a ClearBit() call for a single view. +func (e *Executor) executeClearBitField(ctx context.Context, index string, c *pql.Call, f *Field, colID, rowID uint64, opt *ExecOptions) (bool, error) { slice := colID / SliceWidth ret := false for _, node := range e.Cluster.sliceNodes(index, slice) { // Update locally if host matches. if node.ID == e.Node.ID { - val, err := f.ClearBit(view, rowID, colID, nil) + val, err := f.ClearBit(rowID, colID, nil) if err != nil { return false, err } else if val { @@ -1101,18 +1101,18 @@ func (e *Executor) executeSetBit(ctx context.Context, index string, c *pql.Call, timestamp = &t } - return e.executeSetBitView(ctx, index, c, f, ViewStandard, colID, rowID, timestamp, opt) + return e.executeSetBitField(ctx, index, c, f, colID, rowID, timestamp, opt) } -// executeSetBitView executes a SetBit() call for a specific view. -func (e *Executor) executeSetBitView(ctx context.Context, index string, c *pql.Call, f *Field, view string, colID, rowID uint64, timestamp *time.Time, opt *ExecOptions) (bool, error) { +// executeSetBitField executes a SetBit() call for a specific view. +func (e *Executor) executeSetBitField(ctx context.Context, index string, c *pql.Call, f *Field, colID, rowID uint64, timestamp *time.Time, opt *ExecOptions) (bool, error) { slice := colID / SliceWidth ret := false for _, node := range e.Cluster.sliceNodes(index, slice) { // Update locally if host matches. if node.ID == e.Node.ID { - val, err := f.SetBit(view, rowID, colID, timestamp) + val, err := f.SetBit(rowID, colID, timestamp) if err != nil { return false, err } else if val { diff --git a/field.go b/field.go index d9c88c924..fda0ee364 100644 --- a/field.go +++ b/field.go @@ -654,14 +654,11 @@ func (f *Field) ViewRow(viewName string, rowID uint64) (*Row, error) { } // SetBit sets a bit on a view within the field. -func (f *Field) SetBit(name string, rowID, colID uint64, t *time.Time) (changed bool, err error) { - // Validate view name. - if !isValidView(name) { - return false, ErrInvalidView - } +func (f *Field) SetBit(rowID, colID uint64, t *time.Time) (changed bool, err error) { + viewName := ViewStandard // Retrieve view. Exit if it doesn't exist. - view, err := f.CreateViewIfNotExists(name) + view, err := f.CreateViewIfNotExists(viewName) if err != nil { return changed, errors.Wrap(err, "creating view") } @@ -679,7 +676,7 @@ func (f *Field) SetBit(name string, rowID, colID uint64, t *time.Time) (changed } // If a timestamp is specified then set bits across all views for the quantum. - for _, subname := range viewsByTime(name, *t, f.TimeQuantum()) { + for _, subname := range viewsByTime(viewName, *t, f.TimeQuantum()) { view, err := f.CreateViewIfNotExists(subname) if err != nil { return changed, errors.Wrapf(err, "creating view %s", subname) @@ -696,14 +693,11 @@ func (f *Field) SetBit(name string, rowID, colID uint64, t *time.Time) (changed } // ClearBit clears a bit within the field. -func (f *Field) ClearBit(name string, rowID, colID uint64, t *time.Time) (changed bool, err error) { - // Validate view name. - if !isValidView(name) { - return false, ErrInvalidView - } +func (f *Field) ClearBit(rowID, colID uint64, t *time.Time) (changed bool, err error) { + viewName := ViewStandard // Retrieve view. Exit if it doesn't exist. - view, err := f.CreateViewIfNotExists(name) + view, err := f.CreateViewIfNotExists(viewName) if err != nil { return changed, errors.Wrap(err, "creating view") } @@ -721,7 +715,7 @@ func (f *Field) ClearBit(name string, rowID, colID uint64, t *time.Time) (change } // If a timestamp is specified then clear bits across all views for the quantum. - for _, subname := range viewsByTime(name, *t, f.TimeQuantum()) { + for _, subname := range viewsByTime(viewName, *t, f.TimeQuantum()) { view, err := f.CreateViewIfNotExists(subname) if err != nil { return changed, errors.Wrapf(err, "creating view %s", subname) diff --git a/holder_test.go b/holder_test.go index 72a10b815..c70ffcca6 100644 --- a/holder_test.go +++ b/holder_test.go @@ -210,7 +210,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if field, err := idx.CreateField("bar", pilosa.FieldOptions{}); err != nil { t.Fatal(err) - } else if _, err := field.SetBit(pilosa.ViewStandard, 0, 0, nil); err != nil { + } else if _, err := field.SetBit(0, 0, nil); err != nil { t.Fatal(err) } else if err := h.Holder.Close(); err != nil { t.Fatal(err) @@ -231,7 +231,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if field, err := idx.CreateField("bar", pilosa.FieldOptions{}); err != nil { t.Fatal(err) - } else if _, err := field.SetBit(pilosa.ViewStandard, 0, 0, nil); err != nil { + } else if _, err := field.SetBit(0, 0, nil); err != nil { t.Fatal(err) } else if err := h.Holder.Close(); err != nil { t.Fatal(err) @@ -257,7 +257,7 @@ func TestHolder_Open(t *testing.T) { t.Fatal(err) } else if view, err := field.CreateViewIfNotExists(pilosa.ViewStandard); err != nil { t.Fatal(err) - } else if _, err := field.SetBit(pilosa.ViewStandard, 0, 0, nil); err != nil { + } else if _, err := field.SetBit(0, 0, nil); err != nil { t.Fatal(err) } else if err := view.Fragment(0).FlushCache(); err != nil { t.Fatal(err) diff --git a/http/handler_test.go b/http/handler_test.go index 93f9906b2..2f3ab0aba 100644 --- a/http/handler_test.go +++ b/http/handler_test.go @@ -85,12 +85,12 @@ func TestHandler_Schema(t *testing.T) { if f, err := i0.CreateFieldIfNotExists("f1", pilosa.FieldOptions{}); err != nil { t.Fatal(err) - } else if _, err := f.SetBit(pilosa.ViewStandard, 0, 0, nil); err != nil { + } else if _, err := f.SetBit(0, 0, nil); err != nil { t.Fatal(err) } if f, err := i1.CreateFieldIfNotExists("f0", pilosa.FieldOptions{}); err != nil { t.Fatal(err) - } else if _, err := f.SetBit(pilosa.ViewStandard, 0, 0, nil); err != nil { + } else if _, err := f.SetBit(0, 0, nil); err != nil { t.Fatal(err) } if _, err := i0.CreateFieldIfNotExists("f0", pilosa.FieldOptions{}); err != nil { @@ -122,12 +122,12 @@ func TestHandler_Status(t *testing.T) { if f, err := i0.CreateFieldIfNotExists("f1", pilosa.FieldOptions{}); err != nil { t.Fatal(err) - } else if _, err := f.SetBit(pilosa.ViewStandard, 0, 0, nil); err != nil { + } else if _, err := f.SetBit(0, 0, nil); err != nil { t.Fatal(err) } if f, err := i1.CreateFieldIfNotExists("f0", pilosa.FieldOptions{}); err != nil { t.Fatal(err) - } else if _, err := f.SetBit(pilosa.ViewStandard, 0, 0, nil); err != nil { + } else if _, err := f.SetBit(0, 0, nil); err != nil { t.Fatal(err) } if _, err := i0.CreateFieldIfNotExists("f0", pilosa.FieldOptions{}); err != nil { diff --git a/test/field.go b/test/field.go index 75dc5800d..9a83be2de 100644 --- a/test/field.go +++ b/test/field.go @@ -18,7 +18,6 @@ import ( "io/ioutil" "os" "testing" - "time" "github.com/pilosa/pilosa" ) @@ -75,15 +74,6 @@ func (f *Field) Reopen() error { return nil } -// MustSetBit sets a bit on the field. Panic on error. -func (f *Field) MustSetBit(view string, rowID, columnID uint64, t *time.Time) (changed bool) { - changed, err := f.SetBit(view, rowID, columnID, t) - if err != nil { - panic(err) - } - return changed -} - // Ensure field can set its cache func TestField_SetCacheSize(t *testing.T) { f := MustOpenField() diff --git a/test/holder.go b/test/holder.go index 4484850fd..7bae8afaa 100644 --- a/test/holder.go +++ b/test/holder.go @@ -142,7 +142,7 @@ func (h *Holder) SetBit(index, field string, rowID, columnID uint64) { if err != nil { panic(err) } - f.SetBit(pilosa.ViewStandard, rowID, columnID, nil) + f.SetBit(rowID, columnID, nil) } // ClearBit clears a bit on the given field. @@ -152,7 +152,7 @@ func (h *Holder) ClearBit(index, field string, rowID, columnID uint64) { if err != nil { panic(err) } - f.ClearBit(pilosa.ViewStandard, rowID, columnID, nil) + f.ClearBit(rowID, columnID, nil) } // MustSetBits sets columns on a row. Panic on error. diff --git a/utils_internal_test.go b/utils_internal_test.go index d1b49db03..56340a84b 100644 --- a/utils_internal_test.go +++ b/utils_internal_test.go @@ -117,7 +117,7 @@ func (t *ClusterCluster) CreateField(index, field string, opt FieldOptions) erro return nil } -func (t *ClusterCluster) SetBit(index, field, view string, rowID, colID uint64, x *time.Time) error { +func (t *ClusterCluster) SetBit(index, field string, rowID, colID uint64, x *time.Time) error { // Determine which node should receive the SetBit. c0 := t.Clusters[0] // use the first node's cluster to determine slice location. slice := colID / SliceWidth @@ -132,7 +132,7 @@ func (t *ClusterCluster) SetBit(index, field, view string, rowID, colID uint64, if f == nil { return fmt.Errorf("index/field does not exist: %s/%s", index, field) } - _, err := f.SetBit(view, rowID, colID, x) + _, err := f.SetBit(rowID, colID, x) if err != nil { return err } From 7a05f32a172991edfb3a8bf1ef01bc1c982e77e0 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Wed, 20 Jun 2018 11:14:09 -0500 Subject: [PATCH 08/16] remove NewAttrStore field on Server - unused --- server.go | 4 ---- test/pilosa.go | 3 +-- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/server.go b/server.go index 14d9bb2b6..c544ae6fc 100644 --- a/server.go +++ b/server.go @@ -66,7 +66,6 @@ type Server struct { BroadcastReceiver BroadcastReceiver systemInfo SystemInfo gcNotifier GCNotifier - NewAttrStore func(string) AttrStore logger Logger ln net.Listener @@ -109,7 +108,6 @@ func OptServerDataDir(dir string) ServerOption { func OptServerAttrStoreFunc(af func(string) AttrStore) ServerOption { return func(s *Server) error { - s.NewAttrStore = af s.Holder.NewAttrStore = af return nil } @@ -232,8 +230,6 @@ func NewServer(opts ...ServerOption) (*Server, error) { gcNotifier: NopGCNotifier, - NewAttrStore: NewNopAttrStore, - antiEntropyInterval: time.Minute * 10, metricInterval: 0, diagnosticInterval: 0, diff --git a/test/pilosa.go b/test/pilosa.go index dca6243fb..a65207c91 100644 --- a/test/pilosa.go +++ b/test/pilosa.go @@ -164,8 +164,7 @@ func (m *Main) Reopen() error { return errors.Wrap(err, "setting up server") } - m.Server.NewAttrStore = boltdb.NewAttrStore - m.Server.Holder.NewAttrStore = m.Server.NewAttrStore + m.Server.Holder.NewAttrStore = boltdb.NewAttrStore // Run new program. if err := m.Start(); err != nil { From 44d3e87d4e844d6a125bb6c2ef5c6ced8366c48c Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Wed, 20 Jun 2018 11:21:03 -0500 Subject: [PATCH 09/16] remove remaining external ref to Server.Holder and unexport holder --- diagnostics.go | 2 +- server.go | 88 +++++++++++++++++++++++++------------------------- test/pilosa.go | 3 -- 3 files changed, 45 insertions(+), 48 deletions(-) diff --git a/diagnostics.go b/diagnostics.go index e4067c6e3..b36335b38 100644 --- a/diagnostics.go +++ b/diagnostics.go @@ -222,7 +222,7 @@ func (d *DiagnosticsCollector) EnrichWithSchemaProperties() { bsiFieldCount := 0 timeQuantumEnabled := false - for _, index := range d.server.Holder.Indexes() { + for _, index := range d.server.holder.Indexes() { numSlices += index.MaxSlice() + 1 numIndexes += 1 for _, field := range index.Fields() { diff --git a/server.go b/server.go index c544ae6fc..1d301df35 100644 --- a/server.go +++ b/server.go @@ -52,7 +52,7 @@ type Server struct { closing chan struct{} // Internal - Holder *Holder + holder *Holder Cluster *Cluster TranslateFile *TranslateFile diagnostics *DiagnosticsCollector @@ -108,7 +108,7 @@ func OptServerDataDir(dir string) ServerOption { func OptServerAttrStoreFunc(af func(string) AttrStore) ServerOption { return func(s *Server) error { - s.Holder.NewAttrStore = af + s.holder.NewAttrStore = af return nil } } @@ -180,7 +180,7 @@ func OptServerPrimaryTranslateStore(store TranslateStore) ServerOption { func OptServerStatsClient(sc StatsClient) ServerOption { return func(s *Server) error { - s.Holder.Stats = sc + s.holder.Stats = sc return nil } } @@ -222,7 +222,7 @@ func NewServer(opts ...ServerOption) (*Server, error) { s := &Server{ closing: make(chan struct{}), Cluster: NewCluster(), - Holder: NewHolder(), + holder: NewHolder(), Broadcaster: NopBroadcaster, BroadcastReceiver: NopBroadcastReceiver, diagnostics: NewDiagnosticsCollector(DefaultDiagnosticServer), @@ -250,13 +250,13 @@ func NewServer(opts ...ServerOption) (*Server, error) { return nil, err } - s.Holder.Path = path - s.Holder.Logger = s.logger - s.Holder.Stats.SetLogger(s.logger) + s.holder.Path = path + s.holder.Logger = s.logger + s.holder.Stats.SetLogger(s.logger) s.Cluster.Path = path s.Cluster.Logger = s.logger - s.Cluster.Holder = s.Holder + s.Cluster.Holder = s.holder // Initialize translation database. s.TranslateFile = NewTranslateFile() @@ -288,9 +288,9 @@ func NewServer(opts ...ServerOption) (*Server, error) { } // Append the NodeID tag to stats. - s.Holder.Stats = s.Holder.Stats.WithTags(fmt.Sprintf("NodeID:%s", s.NodeID)) + s.holder.Stats = s.holder.Stats.WithTags(fmt.Sprintf("NodeID:%s", s.NodeID)) - s.executor.Holder = s.Holder + s.executor.Holder = s.holder s.executor.Node = node s.executor.Cluster = s.Cluster s.executor.TranslateStore = s.TranslateFile @@ -309,7 +309,7 @@ func (s *Server) Open() error { } // Log startup - err := s.Holder.logStartup() + err := s.holder.logStartup() if err != nil { log.Println(errors.Wrap(err, "logging startup")) } @@ -320,14 +320,14 @@ func (s *Server) Open() error { // Initialize HTTP handler. api := s.handler.GetAPI() - api.Holder = s.Holder + api.Holder = s.holder api.Broadcaster = s.Broadcaster api.BroadcastHandler = s api.StatusHandler = s api.Cluster = s.Cluster // Initialize Holder. - s.Holder.Broadcaster = s.Broadcaster + s.holder.Broadcaster = s.Broadcaster // Serve handler. go s.handler.Serve(s.ln, s.closing) @@ -343,7 +343,7 @@ func (s *Server) Open() error { } // Open holder. - if err := s.Holder.Open(); err != nil { + if err := s.holder.Open(); err != nil { return fmt.Errorf("opening Holder: %v", err) } if err := s.Cluster.setNodeState(NodeStateReady); err != nil { @@ -378,8 +378,8 @@ func (s *Server) Close() error { if s.Cluster != nil { s.Cluster.close() } - if s.Holder != nil { - s.Holder.Close() + if s.holder != nil { + s.holder.Close() } if s.TranslateFile != nil { s.TranslateFile.Close() @@ -394,7 +394,7 @@ func (s *Server) LoadNodeID() string { if s.NodeID != "" { return s.NodeID } - nodeID, err := s.Holder.loadNodeID() + nodeID, err := s.holder.loadNodeID() if err != nil { s.logger.Printf("loading NodeID: %v", err) return s.NodeID @@ -422,18 +422,18 @@ func (s *Server) monitorAntiEntropy() { case <-s.closing: return case <-ticker.C: - s.Holder.Stats.Count("AntiEntropy", 1, 1.0) + s.holder.Stats.Count("AntiEntropy", 1, 1.0) } t := time.Now() s.logger.Printf("holder sync beginning") // Initialize syncer with local holder and remote client. var syncer HolderSyncer - syncer.Holder = s.Holder + syncer.Holder = s.holder syncer.Node = s.Cluster.Node syncer.Cluster = s.Cluster syncer.Closing = s.closing - syncer.Stats = s.Holder.Stats.WithTags("HolderSyncer") + syncer.Stats = s.holder.Stats.WithTags("HolderSyncer") // Sync holders. if err := syncer.SyncHolder(); err != nil { @@ -444,7 +444,7 @@ func (s *Server) monitorAntiEntropy() { // Record successful sync in log. s.logger.Printf("holder sync complete") dif := time.Since(t) - s.Holder.Stats.Histogram("AntiEntropyDuration", float64(dif), 1.0) + s.holder.Stats.Histogram("AntiEntropyDuration", float64(dif), 1.0) } } @@ -452,23 +452,23 @@ func (s *Server) monitorAntiEntropy() { func (s *Server) ReceiveMessage(pb proto.Message) error { switch obj := pb.(type) { case *internal.CreateSliceMessage: - idx := s.Holder.Index(obj.Index) + idx := s.holder.Index(obj.Index) if idx == nil { return fmt.Errorf("Local Index not found: %s", obj.Index) } idx.SetRemoteMaxSlice(obj.Slice) case *internal.CreateIndexMessage: opt := IndexOptions{} - _, err := s.Holder.CreateIndex(obj.Index, opt) + _, err := s.holder.CreateIndex(obj.Index, opt) if err != nil { return err } case *internal.DeleteIndexMessage: - if err := s.Holder.DeleteIndex(obj.Index); err != nil { + if err := s.holder.DeleteIndex(obj.Index); err != nil { return err } case *internal.CreateFieldMessage: - idx := s.Holder.Index(obj.Index) + idx := s.holder.Index(obj.Index) if idx == nil { return fmt.Errorf("Local Index not found: %s", obj.Index) } @@ -478,12 +478,12 @@ func (s *Server) ReceiveMessage(pb proto.Message) error { return err } case *internal.DeleteFieldMessage: - idx := s.Holder.Index(obj.Index) + idx := s.holder.Index(obj.Index) if err := idx.DeleteField(obj.Field); err != nil { return err } case *internal.CreateViewMessage: - f := s.Holder.Field(obj.Index, obj.Field) + f := s.holder.Field(obj.Index, obj.Field) if f == nil { return fmt.Errorf("Local Field not found: %s", obj.Field) } @@ -492,7 +492,7 @@ func (s *Server) ReceiveMessage(pb proto.Message) error { return err } case *internal.DeleteViewMessage: - f := s.Holder.Field(obj.Index, obj.Field) + f := s.holder.Field(obj.Index, obj.Field) if f == nil { return fmt.Errorf("Local Field not found: %s", obj.Field) } @@ -525,7 +525,7 @@ func (s *Server) ReceiveMessage(pb proto.Message) error { return err } case *internal.RecalculateCaches: - s.Holder.RecalculateCaches() + s.holder.RecalculateCaches() case *internal.NodeEventMessage: s.Cluster.ReceiveEvent(DecodeNodeEvent(obj)) } @@ -577,14 +577,14 @@ func (s *Server) LocalStatus() (proto.Message, error) { if s.Cluster == nil { return nil, errors.New("Server.Cluster is nil") } - if s.Holder == nil { + if s.holder == nil { return nil, errors.New("Server.Holder is nil") } ns := internal.NodeStatus{ Node: EncodeNode(s.Cluster.Node), - MaxSlices: s.Holder.EncodeMaxSlices(), - Schema: s.Holder.EncodeSchema(), + MaxSlices: s.holder.EncodeMaxSlices(), + Schema: s.holder.EncodeSchema(), } return &ns, nil @@ -604,7 +604,7 @@ func (s *Server) HandleRemoteStatus(pb proto.Message) error { go func() { // Make sure the holder has opened. - <-s.Holder.opened + <-s.holder.opened err := s.mergeRemoteStatus(pb.(*internal.NodeStatus)) if err != nil { @@ -622,14 +622,14 @@ func (s *Server) mergeRemoteStatus(ns *internal.NodeStatus) error { } // Sync schema. - if err := s.Holder.ApplySchema(ns.Schema); err != nil { + if err := s.holder.ApplySchema(ns.Schema); err != nil { return errors.Wrap(err, "applying schema") } // Sync maxSlices. - oldmaxslices := s.Holder.MaxSlices() + oldmaxslices := s.holder.MaxSlices() for index, newMax := range ns.MaxSlices.Standard { - localIndex := s.Holder.Index(index) + localIndex := s.holder.Index(index) // if we don't know about an index locally, log an error because // indexes should be created and synced prior to slice creation if localIndex == nil { @@ -717,26 +717,26 @@ func (s *Server) monitorRuntime() { return case <-s.gcNotifier.AfterGC(): // GC just ran. - s.Holder.Stats.Count("garbage_collection", 1, 1.0) + s.holder.Stats.Count("garbage_collection", 1, 1.0) case <-ticker.C: } // Record the number of go routines. - s.Holder.Stats.Gauge("goroutines", float64(runtime.NumGoroutine()), 1.0) + s.holder.Stats.Gauge("goroutines", float64(runtime.NumGoroutine()), 1.0) openFiles, err := countOpenFiles() // Open File handles. if err == nil { - s.Holder.Stats.Gauge("OpenFiles", float64(openFiles), 1.0) + s.holder.Stats.Gauge("OpenFiles", float64(openFiles), 1.0) } // Runtime memory metrics. runtime.ReadMemStats(&m) - s.Holder.Stats.Gauge("HeapAlloc", float64(m.HeapAlloc), 1.0) - s.Holder.Stats.Gauge("HeapInuse", float64(m.HeapInuse), 1.0) - s.Holder.Stats.Gauge("StackInuse", float64(m.StackInuse), 1.0) - s.Holder.Stats.Gauge("Mallocs", float64(m.Mallocs), 1.0) - s.Holder.Stats.Gauge("Frees", float64(m.Frees), 1.0) + s.holder.Stats.Gauge("HeapAlloc", float64(m.HeapAlloc), 1.0) + s.holder.Stats.Gauge("HeapInuse", float64(m.HeapInuse), 1.0) + s.holder.Stats.Gauge("StackInuse", float64(m.StackInuse), 1.0) + s.holder.Stats.Gauge("Mallocs", float64(m.Mallocs), 1.0) + s.holder.Stats.Gauge("Frees", float64(m.Frees), 1.0) } } diff --git a/test/pilosa.go b/test/pilosa.go index a65207c91..bd1628aa6 100644 --- a/test/pilosa.go +++ b/test/pilosa.go @@ -25,7 +25,6 @@ import ( "testing" "time" - "github.com/pilosa/pilosa/boltdb" "github.com/pilosa/pilosa/gossip" "github.com/pilosa/pilosa/http" "github.com/pilosa/pilosa/server" @@ -164,8 +163,6 @@ func (m *Main) Reopen() error { return errors.Wrap(err, "setting up server") } - m.Server.Holder.NewAttrStore = boltdb.NewAttrStore - // Run new program. if err := m.Start(); err != nil { return err From d529ee3ccc5661068116688abf07d9ee09d4d32c Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Wed, 20 Jun 2018 11:26:33 -0500 Subject: [PATCH 10/16] unexport Server.TranslateFile --- server.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/server.go b/server.go index 1d301df35..16b2f3747 100644 --- a/server.go +++ b/server.go @@ -54,7 +54,7 @@ type Server struct { // Internal holder *Holder Cluster *Cluster - TranslateFile *TranslateFile + translateFile *TranslateFile diagnostics *DiagnosticsCollector executor *Executor hosts []string @@ -259,10 +259,10 @@ func NewServer(opts ...ServerOption) (*Server, error) { s.Cluster.Holder = s.holder // Initialize translation database. - s.TranslateFile = NewTranslateFile() - s.TranslateFile.Path = filepath.Join(path, "keys") - s.TranslateFile.PrimaryTranslateStore = s.primaryTranslateStore - if err := s.TranslateFile.Open(); err != nil { + s.translateFile = NewTranslateFile() + s.translateFile.Path = filepath.Join(path, "keys") + s.translateFile.PrimaryTranslateStore = s.primaryTranslateStore + if err := s.translateFile.Open(); err != nil { return nil, err } @@ -293,10 +293,10 @@ func NewServer(opts ...ServerOption) (*Server, error) { s.executor.Holder = s.holder s.executor.Node = node s.executor.Cluster = s.Cluster - s.executor.TranslateStore = s.TranslateFile + s.executor.TranslateStore = s.translateFile s.executor.MaxWritesPerRequest = s.maxWritesPerRequest s.handler.GetAPI().Executor = s.executor - s.handler.GetAPI().TranslateStore = s.TranslateFile + s.handler.GetAPI().TranslateStore = s.translateFile return s, nil } @@ -381,8 +381,8 @@ func (s *Server) Close() error { if s.holder != nil { s.holder.Close() } - if s.TranslateFile != nil { - s.TranslateFile.Close() + if s.translateFile != nil { + s.translateFile.Close() } return nil From b271ff286ce4353a887eb3a18c04479f13079851 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Wed, 20 Jun 2018 11:31:30 -0500 Subject: [PATCH 11/16] unexport done channel on server.Command --- server/server.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/server.go b/server/server.go index 4898088f3..924a46d10 100644 --- a/server/server.go +++ b/server/server.go @@ -65,10 +65,10 @@ type Command struct { // Standard input/output *pilosa.CmdIO - // Started will be closed once Command.Run is finished. + // Started will be closed once Command.Start is finished. Started chan struct{} - // Done will be closed when Command.Close() is called - Done chan struct{} + // done will be closed when Command.Close() is called + done chan struct{} // Passed to the Gossip implementation. logOutput io.Writer @@ -83,7 +83,7 @@ func NewCommand(stdin io.Reader, stdout, stderr io.Writer) *Command { CmdIO: pilosa.NewCmdIO(stdin, stdout, stderr), Started: make(chan struct{}), - Done: make(chan struct{}), + done: make(chan struct{}), } } @@ -125,7 +125,7 @@ func (m *Command) Wait() error { // Second signal causes a hard shutdown. go func() { <-c; os.Exit(1) }() return errors.Wrap(m.Close(), "closing command") - case <-m.Done: + case <-m.done: m.logger.Printf("Server closed externally") return nil } @@ -305,7 +305,7 @@ func (m *Command) Close() error { if closer, ok := m.logOutput.(io.Closer); ok { logErr = closer.Close() } - close(m.Done) + close(m.done) if serveErr != nil && logErr != nil { return fmt.Errorf("closing server: '%v', closing logs: '%v'", serveErr, logErr) } else if logErr != nil { From 343880e0adba04449118442ccc38f6a735ab1ebe Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Wed, 20 Jun 2018 13:03:53 -0500 Subject: [PATCH 12/16] remove broadcaster from server --- cluster.go | 3 +++ server.go | 8 +++----- server/server.go | 1 - 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cluster.go b/cluster.go index c8cec60a2..0f998507e 100644 --- a/cluster.go +++ b/cluster.go @@ -1010,6 +1010,9 @@ func (c *Cluster) handleNodeAction(nodeAction nodeAction) error { func (c *Cluster) setStateAndBroadcast(state string) error { c.SetState(state) + if c.Static { + return nil + } // Broadcast cluster status changes to the cluster. c.Logger.Printf("broadcasting ClusterStatus: %s", state) return c.Broadcaster.SendSync(c.Status()) diff --git a/server.go b/server.go index 16b2f3747..b6c4e7996 100644 --- a/server.go +++ b/server.go @@ -62,7 +62,6 @@ type Server struct { // External handler Handler - Broadcaster Broadcaster BroadcastReceiver BroadcastReceiver systemInfo SystemInfo gcNotifier GCNotifier @@ -223,7 +222,6 @@ func NewServer(opts ...ServerOption) (*Server, error) { closing: make(chan struct{}), Cluster: NewCluster(), holder: NewHolder(), - Broadcaster: NopBroadcaster, BroadcastReceiver: NopBroadcastReceiver, diagnostics: NewDiagnosticsCollector(DefaultDiagnosticServer), systemInfo: NewNopSystemInfo(), @@ -315,19 +313,19 @@ func (s *Server) Open() error { } // Cluster settings. - s.Cluster.Broadcaster = s.Broadcaster + s.Cluster.Broadcaster = s s.Cluster.MaxWritesPerRequest = s.maxWritesPerRequest // Initialize HTTP handler. api := s.handler.GetAPI() api.Holder = s.holder - api.Broadcaster = s.Broadcaster + api.Broadcaster = s api.BroadcastHandler = s api.StatusHandler = s api.Cluster = s.Cluster // Initialize Holder. - s.holder.Broadcaster = s.Broadcaster + s.holder.Broadcaster = s // Serve handler. go s.handler.Serve(s.ln, s.closing) diff --git a/server/server.go b/server/server.go index 924a46d10..d97e36520 100644 --- a/server/server.go +++ b/server/server.go @@ -293,7 +293,6 @@ func (m *Command) SetupNetworking() error { } gossipMemberSet.Logger = m.logger m.Server.Cluster.MemberSet = gossipMemberSet - m.Server.Broadcaster = m.Server m.Server.BroadcastReceiver = gossipMemberSet return nil } From aac2397949784a6f33795dd3afeaa827be45cfb2 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 20 Jun 2018 13:13:46 -0500 Subject: [PATCH 13/16] enforced Accept for json response endpoints --- ctl/import_test.go | 1 + http/client.go | 7 ++++ http/handler.go | 79 ++++++++++++++++++++++++++++++++++++++++++-- http/handler_test.go | 20 ++++++++--- test/handler.go | 1 + test/pilosa.go | 13 +++++--- test/pilosa_test.go | 16 +++++++-- 7 files changed, 124 insertions(+), 13 deletions(-) diff --git a/ctl/import_test.go b/ctl/import_test.go index ba8a9dc6e..2284ca873 100644 --- a/ctl/import_test.go +++ b/ctl/import_test.go @@ -183,6 +183,7 @@ func TestImportCommand_InvalidFile(t *testing.T) { // MustNewHTTPRequest creates a new HTTP request. Panic on error. func MustNewHTTPRequest(method, urlStr string, body io.Reader) *http.Request { req, err := http.NewRequest(method, urlStr, body) + req.Header.Add("Accept", "application/json") if err != nil { panic(err) } diff --git a/http/client.go b/http/client.go index d8f0d290d..5090072a3 100644 --- a/http/client.go +++ b/http/client.go @@ -90,6 +90,7 @@ func (c *InternalClient) maxSliceByIndex(ctx context.Context) (map[string]uint64 } req.Header.Set("User-Agent", "pilosa/"+pilosa.Version) + req.Header.Set("Accept", "application/json") // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) @@ -120,6 +121,7 @@ func (c *InternalClient) Schema(ctx context.Context) ([]*pilosa.IndexInfo, error } req.Header.Set("User-Agent", "pilosa/"+pilosa.Version) + req.Header.Set("Accept", "application/json") // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) @@ -195,6 +197,7 @@ func (c *InternalClient) FragmentNodes(ctx context.Context, index string, slice } req.Header.Set("User-Agent", "pilosa/"+pilosa.Version) + req.Header.Set("Accept", "application/json") // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) @@ -685,6 +688,7 @@ func (c *InternalClient) FragmentBlocks(ctx context.Context, uri *pilosa.URI, in } req.Header.Set("User-Agent", "pilosa/"+pilosa.Version) + req.Header.Set("Accept", "application/json") // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) @@ -777,6 +781,7 @@ func (c *InternalClient) ColumnAttrDiff(ctx context.Context, uri *pilosa.URI, in } req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", "pilosa/"+pilosa.Version) + req.Header.Set("Accept", "application/json") // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) @@ -820,6 +825,7 @@ func (c *InternalClient) RowAttrDiff(ctx context.Context, uri *pilosa.URI, index } req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", "pilosa/"+pilosa.Version) + req.Header.Set("Accept", "application/json") // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) @@ -859,6 +865,7 @@ func (c *InternalClient) SendMessage(ctx context.Context, uri *pilosa.URI, pb pr } req.Header.Set("Content-Type", "application/x-protobuf") req.Header.Set("User-Agent", "pilosa/"+pilosa.Version) + req.Header.Set("Accept", "application/json") // Execute request. resp, err := c.HTTPClient.Do(req.WithContext(ctx)) diff --git a/http/handler.go b/http/handler.go index 509378d6e..1c389b2e8 100644 --- a/http/handler.go +++ b/http/handler.go @@ -164,11 +164,12 @@ func (h *Handler) queryArgValidator(next http.Handler) http.Handler { func NewRouter(handler *Handler) *mux.Router { router := mux.NewRouter() router.HandleFunc("/", handler.handleHome).Methods("GET") - router.HandleFunc("/cluster/message", handler.handlePostClusterMessage).Methods("POST") - router.HandleFunc("/cluster/resize/set-coordinator", handler.handlePostClusterResizeSetCoordinator).Methods("POST") router.PathPrefix("/debug/pprof/").Handler(http.DefaultServeMux).Methods("GET") router.Handle("/debug/vars", expvar.Handler()).Methods("GET") + router.HandleFunc("/schema", handler.handleGetSchema).Methods("GET") + router.HandleFunc("/cluster/message", handler.handlePostClusterMessage).Methods("POST") + router.HandleFunc("/cluster/resize/set-coordinator", handler.handlePostClusterResizeSetCoordinator).Methods("POST") router.HandleFunc("/slices/max", handler.handleGetSlicesMax).Methods("GET") // TODO: deprecate, but it's being used by the client router.HandleFunc("/status", handler.handleGetStatus).Methods("GET") router.HandleFunc("/info", handler.handleGetInfo).Methods("GET") @@ -259,6 +260,11 @@ func (h *Handler) handleHome(w http.ResponseWriter, r *http.Request) { // handleGetSchema handles GET /schema requests. func (h *Handler) handleGetSchema(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } + schema := h.API.Schema(r.Context()) if err := json.NewEncoder(w).Encode(getSchemaResponse{ Indexes: schema, @@ -269,6 +275,10 @@ func (h *Handler) handleGetSchema(w http.ResponseWriter, r *http.Request) { // handleGetStatus handles GET /status requests. func (h *Handler) handleGetStatus(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } status := getStatusResponse{ State: h.API.State(), Nodes: h.API.Hosts(r.Context()), @@ -280,6 +290,10 @@ func (h *Handler) handleGetStatus(w http.ResponseWriter, r *http.Request) { } func (h *Handler) handleGetInfo(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } info := h.API.Info() if err := json.NewEncoder(w).Encode(info); err != nil { h.Logger.Printf("write info response error: %s", err) @@ -333,6 +347,10 @@ func (h *Handler) handlePostQuery(w http.ResponseWriter, r *http.Request) { // handleGetSlicesMax handles GET /schema requests. func (h *Handler) handleGetSlicesMax(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } if err := json.NewEncoder(w).Encode(getSlicesMaxResponse{ Standard: h.API.MaxSlices(r.Context()), }); err != nil { @@ -351,6 +369,10 @@ func (h *Handler) handleGetIndexes(w http.ResponseWriter, r *http.Request) { // handleGetIndex handles GET /index/ requests. func (h *Handler) handleGetIndex(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } indexName := mux.Vars(r)["index"] for _, idx := range h.API.Schema(r.Context()) { if idx.Name == indexName { @@ -429,6 +451,10 @@ type postIndexResponse struct{} // handleDeleteIndex handles DELETE /index request. func (h *Handler) handleDeleteIndex(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } indexName := mux.Vars(r)["index"] err := h.API.DeleteIndex(r.Context(), indexName) if err != nil { @@ -447,6 +473,10 @@ type deleteIndexResponse struct{} // handlePostIndex handles POST /index request. func (h *Handler) handlePostIndex(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } indexName := mux.Vars(r)["index"] // Decode request. @@ -477,6 +507,10 @@ func (h *Handler) handlePostIndex(w http.ResponseWriter, r *http.Request) { // handlePostIndexAttrDiff handles POST /index/attr/diff requests. func (h *Handler) handlePostIndexAttrDiff(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } indexName := mux.Vars(r)["index"] // Decode request. @@ -514,6 +548,10 @@ type postIndexAttrDiffResponse struct { // handlePostField handles POST /field request. func (h *Handler) handlePostField(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } indexName := mux.Vars(r)["index"] fieldName := mux.Vars(r)["field"] @@ -592,6 +630,11 @@ type postFieldResponse struct{} // handleDeleteField handles DELETE /field request. func (h *Handler) handleDeleteField(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } + indexName := mux.Vars(r)["index"] fieldName := mux.Vars(r)["field"] @@ -617,6 +660,10 @@ type deleteFieldResponse struct{} // handlePostFieldAttrDiff handles POST /field/attr/diff requests. func (h *Handler) handlePostFieldAttrDiff(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } indexName := mux.Vars(r)["index"] fieldName := mux.Vars(r)["field"] @@ -872,6 +919,10 @@ func (h *Handler) handleGetExportCSV(w http.ResponseWriter, r *http.Request) { // handleGetFragmentNodes handles /fragment/nodes requests. func (h *Handler) handleGetFragmentNodes(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } q := r.URL.Query() index := q.Get("index") @@ -917,6 +968,10 @@ func (h *Handler) handleGetFragmentBlockData(w http.ResponseWriter, r *http.Requ // handleGetFragmentBlocks handles GET /fragment/blocks requests. func (h *Handler) handleGetFragmentBlocks(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } // Read slice parameter. q := r.URL.Query() slice, err := strconv.ParseUint(q.Get("slice"), 10, 64) @@ -949,6 +1004,10 @@ type getFragmentBlocksResponse struct { // handleGetVersion handles /version requests. func (h *Handler) handleGetVersion(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } err := json.NewEncoder(w).Encode(struct { Version string `json:"version"` }{ @@ -1047,6 +1106,10 @@ func errorString(err error) string { } func (h *Handler) handlePostClusterResizeSetCoordinator(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } // Decode request. var req setCoordinatorRequest err := json.NewDecoder(r.Body).Decode(&req) @@ -1084,6 +1147,10 @@ type setCoordinatorResponse struct { // handlePostClusterResizeRemoveNode handles POST /cluster/resize/remove-node request. func (h *Handler) handlePostClusterResizeRemoveNode(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } // Decode request. var req removeNodeRequest err := json.NewDecoder(r.Body).Decode(&req) @@ -1120,6 +1187,10 @@ type removeNodeResponse struct { // handlePostClusterResizeAbort handles POST /cluster/resize/abort request. func (h *Handler) handlePostClusterResizeAbort(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } err := h.API.ResizeAbort() var msg string if err != nil { @@ -1157,6 +1228,10 @@ func (h *Handler) handleRecalculateCaches(w http.ResponseWriter, r *http.Request } func (h *Handler) handlePostClusterMessage(w http.ResponseWriter, r *http.Request) { + if !strings.Contains(r.Header.Get("Accept"), "application/json") { + http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) + return + } // Verify that request is only communicating over protobufs. if r.Header.Get("Content-Type") != "application/x-protobuf" { http.Error(w, "Unsupported media type", http.StatusUnsupportedMediaType) diff --git a/http/handler_test.go b/http/handler_test.go index 93f9906b2..b8ed853f5 100644 --- a/http/handler_test.go +++ b/http/handler_test.go @@ -750,11 +750,17 @@ func TestHandler_Index_AttrStore_Diff(t *testing.T) { blks[1].Checksum = []byte("MISMATCHED_CHECKSUM") // Send block checksums to determine diff. - resp, err := gohttp.Post( + req, err := gohttp.NewRequest( + "POST", s.URL+"/index/i/attr/diff", - "application/json", strings.NewReader(`{"blocks":`+string(test.MustMarshalJSON(blks))+`}`), ) + + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json") + + client := &gohttp.Client{} + resp, err := client.Do(req) if err != nil { t.Fatal(err) } @@ -800,11 +806,17 @@ func TestHandler_Field_AttrStore_Diff(t *testing.T) { blks[1].Checksum = []byte("MISMATCHED_CHECKSUM") // Send block checksums to determine diff. - resp, err := gohttp.Post( + req, err := gohttp.NewRequest( + "POST", s.URL+"/index/i/field/meta/attr/diff", - "application/json", strings.NewReader(`{"blocks":`+string(test.MustMarshalJSON(blks))+`}`), ) + + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json") + + client := &gohttp.Client{} + resp, err := client.Do(req) if err != nil { t.Fatal(err) } diff --git a/test/handler.go b/test/handler.go index 27fa20503..5256413b5 100644 --- a/test/handler.go +++ b/test/handler.go @@ -148,6 +148,7 @@ func MustParseURLHost(rawurl string) string { // MustNewHTTPRequest creates a new HTTP request. Panic on error. func MustNewHTTPRequest(method, urlStr string, body io.Reader) *gohttp.Request { req, err := gohttp.NewRequest(method, urlStr, body) + req.Header.Add("Accept", "application/json") if err != nil { panic(err) } diff --git a/test/pilosa.go b/test/pilosa.go index dca6243fb..e9bb593ba 100644 --- a/test/pilosa.go +++ b/test/pilosa.go @@ -267,10 +267,15 @@ func (m *Main) RecalculateCaches() error { // MustDo executes http.Do() with an http.NewRequest(). Panic on error. func MustDo(method, urlStr string, body string) *httpResponse { - req, err := gohttp.NewRequest(method, urlStr, strings.NewReader(body)) - if err != nil { - panic(err) - } + req, err := gohttp.NewRequest( + method, + urlStr, + strings.NewReader(body), + ) + + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json") + resp, err := gohttp.DefaultClient.Do(req) if err != nil { panic(err) diff --git a/test/pilosa_test.go b/test/pilosa_test.go index 583a244c0..229fde5c0 100644 --- a/test/pilosa_test.go +++ b/test/pilosa_test.go @@ -17,6 +17,7 @@ package test_test import ( "encoding/json" "net/http" + "strings" "testing" "github.com/pilosa/pilosa" @@ -32,12 +33,21 @@ func TestNewCluster(t *testing.T) { t.Fatalf("node %d does not have the same coordinator as node 0. '%v' and '%v' respectively", i, coordi, coordinator) } } + req, err := http.NewRequest( + "GET", + "http://"+cluster[0].Server.Addr().String()+"/status", + strings.NewReader(""), + ) - response, err := http.Get("http://" + cluster[0].Server.Addr().String() + "/status") + req.Header.Set("Accept", "application/json") + + resp, err := http.DefaultClient.Do(req) if err != nil { - t.Fatalf("getting schema: %v", err) + panic(err) } - dec := json.NewDecoder(response.Body) + defer resp.Body.Close() + + dec := json.NewDecoder(resp.Body) body := struct { State string Nodes []struct { From 7da9242b6b6ddc07c3511c25c9603f8b28f70ec6 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 20 Jun 2018 14:39:15 -0500 Subject: [PATCH 14/16] error only on if provided accept not json --- http/handler.go | 56 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/http/handler.go b/http/handler.go index 1c389b2e8..c15780923 100644 --- a/http/handler.go +++ b/http/handler.go @@ -258,9 +258,27 @@ func (h *Handler) handleHome(w http.ResponseWriter, r *http.Request) { http.Error(w, "Welcome. Pilosa is running. Visit https://www.pilosa.com/docs/ for more information.", http.StatusNotFound) } +func checkHeaderAcceptJSON(header http.Header) bool { + + v, found := header["Accept"] + sendError := false + if found { + sendError = true + for _, v := range v { + if v == "application/json" { + sendError = false + + } + } + + } + return sendError + +} + // handleGetSchema handles GET /schema requests. func (h *Handler) handleGetSchema(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -275,7 +293,7 @@ func (h *Handler) handleGetSchema(w http.ResponseWriter, r *http.Request) { // handleGetStatus handles GET /status requests. func (h *Handler) handleGetStatus(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -290,7 +308,7 @@ func (h *Handler) handleGetStatus(w http.ResponseWriter, r *http.Request) { } func (h *Handler) handleGetInfo(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -347,7 +365,7 @@ func (h *Handler) handlePostQuery(w http.ResponseWriter, r *http.Request) { // handleGetSlicesMax handles GET /schema requests. func (h *Handler) handleGetSlicesMax(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -369,7 +387,7 @@ func (h *Handler) handleGetIndexes(w http.ResponseWriter, r *http.Request) { // handleGetIndex handles GET /index/ requests. func (h *Handler) handleGetIndex(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -451,7 +469,7 @@ type postIndexResponse struct{} // handleDeleteIndex handles DELETE /index request. func (h *Handler) handleDeleteIndex(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -473,7 +491,7 @@ type deleteIndexResponse struct{} // handlePostIndex handles POST /index request. func (h *Handler) handlePostIndex(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -507,7 +525,7 @@ func (h *Handler) handlePostIndex(w http.ResponseWriter, r *http.Request) { // handlePostIndexAttrDiff handles POST /index/attr/diff requests. func (h *Handler) handlePostIndexAttrDiff(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -548,7 +566,7 @@ type postIndexAttrDiffResponse struct { // handlePostField handles POST /field request. func (h *Handler) handlePostField(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -630,7 +648,7 @@ type postFieldResponse struct{} // handleDeleteField handles DELETE /field request. func (h *Handler) handleDeleteField(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -660,7 +678,7 @@ type deleteFieldResponse struct{} // handlePostFieldAttrDiff handles POST /field/attr/diff requests. func (h *Handler) handlePostFieldAttrDiff(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -756,7 +774,7 @@ func (h *Handler) readURLQueryRequest(r *http.Request) (*pilosa.QueryRequest, er // writeQueryResponse writes the response from the executor to w. func (h *Handler) writeQueryResponse(w http.ResponseWriter, r *http.Request, resp *pilosa.QueryResponse) error { - if strings.Contains(r.Header.Get("Accept"), "application/x-protobuf") { + if checkHeaderAcceptJSON(r.Header) { return h.writeProtobufQueryResponse(w, resp) } return h.writeJSONQueryResponse(w, resp) @@ -919,7 +937,7 @@ func (h *Handler) handleGetExportCSV(w http.ResponseWriter, r *http.Request) { // handleGetFragmentNodes handles /fragment/nodes requests. func (h *Handler) handleGetFragmentNodes(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -968,7 +986,7 @@ func (h *Handler) handleGetFragmentBlockData(w http.ResponseWriter, r *http.Requ // handleGetFragmentBlocks handles GET /fragment/blocks requests. func (h *Handler) handleGetFragmentBlocks(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -1004,7 +1022,7 @@ type getFragmentBlocksResponse struct { // handleGetVersion handles /version requests. func (h *Handler) handleGetVersion(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -1106,7 +1124,7 @@ func errorString(err error) string { } func (h *Handler) handlePostClusterResizeSetCoordinator(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -1147,7 +1165,7 @@ type setCoordinatorResponse struct { // handlePostClusterResizeRemoveNode handles POST /cluster/resize/remove-node request. func (h *Handler) handlePostClusterResizeRemoveNode(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -1187,7 +1205,7 @@ type removeNodeResponse struct { // handlePostClusterResizeAbort handles POST /cluster/resize/abort request. func (h *Handler) handlePostClusterResizeAbort(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } @@ -1228,7 +1246,7 @@ func (h *Handler) handleRecalculateCaches(w http.ResponseWriter, r *http.Request } func (h *Handler) handlePostClusterMessage(w http.ResponseWriter, r *http.Request) { - if !strings.Contains(r.Header.Get("Accept"), "application/json") { + if checkHeaderAcceptJSON(r.Header) { http.Error(w, "JSON only acceptable response", http.StatusNotAcceptable) return } From 289bec9d81be5912e72d80c2b01c79d868bc8864 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 20 Jun 2018 15:12:26 -0500 Subject: [PATCH 15/16] repace panic with fatal for consistancy --- test/pilosa_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/pilosa_test.go b/test/pilosa_test.go index 229fde5c0..0a1acc551 100644 --- a/test/pilosa_test.go +++ b/test/pilosa_test.go @@ -43,7 +43,7 @@ func TestNewCluster(t *testing.T) { resp, err := http.DefaultClient.Do(req) if err != nil { - panic(err) + t.Fatalf("sending request: %v", err) } defer resp.Body.Close() From d9369ed06e0110fc151472f8410fdf064e4bb3a8 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Wed, 20 Jun 2018 15:29:10 -0500 Subject: [PATCH 16/16] removed whitespace --- http/handler.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/http/handler.go b/http/handler.go index c15780923..674381905 100644 --- a/http/handler.go +++ b/http/handler.go @@ -259,7 +259,6 @@ func (h *Handler) handleHome(w http.ResponseWriter, r *http.Request) { } func checkHeaderAcceptJSON(header http.Header) bool { - v, found := header["Accept"] sendError := false if found { @@ -270,10 +269,8 @@ func checkHeaderAcceptJSON(header http.Header) bool { } } - } return sendError - } // handleGetSchema handles GET /schema requests.