From e03dee938677c9e008f462ad5cfd2eeb9cf4ff5b Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Tue, 6 Mar 2018 16:57:45 -0600 Subject: [PATCH 1/5] put Statik behind an interface --- filesystem.go | 42 ++++++++++++++++++++++++++++++++++++++++++ filesystem/statik.go | 32 ++++++++++++++++++++++++++++++++ handler.go | 10 ++++++++-- handler_test.go | 2 ++ server/server.go | 4 ++++ 5 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 filesystem.go create mode 100644 filesystem/statik.go diff --git a/filesystem.go b/filesystem.go new file mode 100644 index 000000000..0b5b27220 --- /dev/null +++ b/filesystem.go @@ -0,0 +1,42 @@ +// Copyright 2017 Pilosa Corp. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pilosa + +import ( + "fmt" + "net/http" +) + +// Ensure nopStaticFileSystem implements interface. +var _ StaticFileSystem = &nopStaticFileSystem{} + +// StaticFileSystem represents an interface for a static WebUI. +type StaticFileSystem interface { + New() (http.FileSystem, error) +} + +func init() { + NopStaticFileSystem = &nopStaticFileSystem{} +} + +// NopStaticFileSystem represents a StaticFileSystem that returns an error if called. +var NopStaticFileSystem StaticFileSystem + +type nopStaticFileSystem struct{} + +// New is a no-op implementation of StaticFileSystem New method. +func (n *nopStaticFileSystem) New() (http.FileSystem, error) { + return nil, fmt.Errorf("static file system not implemented") +} diff --git a/filesystem/statik.go b/filesystem/statik.go new file mode 100644 index 000000000..405e31a11 --- /dev/null +++ b/filesystem/statik.go @@ -0,0 +1,32 @@ +// Copyright 2017 Pilosa Corp. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package filesystem + +import ( + "net/http" + + "github.com/pilosa/pilosa" + "github.com/rakyll/statik/fs" +) + +// Ensure nopStaticFileSystem implements interface. +var _ pilosa.StaticFileSystem = &StatikFS{} + +type StatikFS struct{} + +// New is a statik implementation of StaticFileSystem New method. +func (s *StatikFS) New() (http.FileSystem, error) { + return fs.New() +} diff --git a/handler.go b/handler.go index 38fd24eca..1688cf5a3 100644 --- a/handler.go +++ b/handler.go @@ -47,7 +47,6 @@ import ( // Allow building Pilosa without the web UI. _ "github.com/pilosa/pilosa/statik" - "github.com/rakyll/statik/fs" ) // Handler represents an HTTP handler. @@ -57,6 +56,8 @@ type Handler struct { BroadcastHandler BroadcastHandler StatusHandler StatusHandler + StaticFileSystem StaticFileSystem + // Local hostname & cluster configuration. Node *Node Cluster *Cluster @@ -98,6 +99,11 @@ type errorResponse struct { // NewHandler returns a new instance of Handler with a default logger. func NewHandler() *Handler { handler := &Handler{ + Broadcaster: NopBroadcaster, + //BroadcastHandler: NopBroadcastHandler, // TODO: implement the nop + //StatusHandler: NopStatusHandler, // TODO: implement the nop + StaticFileSystem: NopStaticFileSystem, + LogOutput: os.Stderr, } BuildRouters(handler) @@ -285,7 +291,7 @@ func (h *Handler) handleWebUI(w http.ResponseWriter, r *http.Request) { http.Error(w, "Welcome. Pilosa is running. Visit https://www.pilosa.com/docs/ for more information or try the WebUI by visiting this URL in your browser.", http.StatusNotFound) return } - statikFS, err := fs.New() + statikFS, err := h.StaticFileSystem.New() if err != nil { h.writeQueryResponse(w, r, &QueryResponse{Err: err}) h.logger().Println("Pilosa WebUI is not available. Please run `make generate-statik` before building Pilosa with `make install`.") diff --git a/handler_test.go b/handler_test.go index 1caf7bf99..6386f53f9 100644 --- a/handler_test.go +++ b/handler_test.go @@ -29,6 +29,7 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa" + "github.com/pilosa/pilosa/filesystem" "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/pql" "github.com/pilosa/pilosa/test" @@ -1853,6 +1854,7 @@ func TestHandler_WebUI(t *testing.T) { h := test.NewHandler() h.Holder = hldr.Holder h.Cluster = test.NewCluster(1) + h.StaticFileSystem = &filesystem.StatikFS{} w := httptest.NewRecorder() h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/", nil)) diff --git a/server/server.go b/server/server.go index f0d3314fa..e056bc76d 100644 --- a/server/server.go +++ b/server/server.go @@ -33,6 +33,7 @@ import ( "crypto/tls" "github.com/pilosa/pilosa" + "github.com/pilosa/pilosa/filesystem" "github.com/pilosa/pilosa/gcnotify" "github.com/pilosa/pilosa/gossip" "github.com/pilosa/pilosa/statsd" @@ -191,6 +192,9 @@ func (m *Command) SetupServer() error { m.Server.Handler.RemoteClient = c m.Server.Cluster.RemoteClient = c + // Statik file system. + m.Server.Handler.StaticFileSystem = &filesystem.StatikFS{} + // Default coordintor to port 0 when not specified so that coordinator // can be set to the value of server.URI after server binds to a port. // This would only be useful in a one-node cluster. From d6896ea0a5127489cf800cfa0a3d04cf074f1c3d Mon Sep 17 00:00:00 2001 From: Travis Turner Date: Wed, 7 Mar 2018 11:12:05 -0600 Subject: [PATCH 2/5] rename StaticFileSystem to FileSystem. re-org statik files --- filesystem.go | 22 +++++++++---------- handler.go | 6 ++--- handler_test.go | 4 ++-- server/server.go | 4 ++-- .../statik.go => statikfs/filesystem.go | 13 ++++++----- 5 files changed, 25 insertions(+), 24 deletions(-) rename filesystem/statik.go => statikfs/filesystem.go (70%) diff --git a/filesystem.go b/filesystem.go index 0b5b27220..5664f0987 100644 --- a/filesystem.go +++ b/filesystem.go @@ -19,24 +19,24 @@ import ( "net/http" ) -// Ensure nopStaticFileSystem implements interface. -var _ StaticFileSystem = &nopStaticFileSystem{} +// Ensure nopFileSystem implements interface. +var _ FileSystem = &nopFileSystem{} -// StaticFileSystem represents an interface for a static WebUI. -type StaticFileSystem interface { +// FileSystem represents an interface for a WebUI file system. +type FileSystem interface { New() (http.FileSystem, error) } func init() { - NopStaticFileSystem = &nopStaticFileSystem{} + NopFileSystem = &nopFileSystem{} } -// NopStaticFileSystem represents a StaticFileSystem that returns an error if called. -var NopStaticFileSystem StaticFileSystem +// NopFileSystem represents a FileSystem that returns an error if called. +var NopFileSystem FileSystem -type nopStaticFileSystem struct{} +type nopFileSystem struct{} -// New is a no-op implementation of StaticFileSystem New method. -func (n *nopStaticFileSystem) New() (http.FileSystem, error) { - return nil, fmt.Errorf("static file system not implemented") +// New is a no-op implementation of FileSystem New method. +func (n *nopFileSystem) New() (http.FileSystem, error) { + return nil, fmt.Errorf("file system not implemented") } diff --git a/handler.go b/handler.go index 1688cf5a3..067219c8f 100644 --- a/handler.go +++ b/handler.go @@ -56,7 +56,7 @@ type Handler struct { BroadcastHandler BroadcastHandler StatusHandler StatusHandler - StaticFileSystem StaticFileSystem + FileSystem FileSystem // Local hostname & cluster configuration. Node *Node @@ -102,7 +102,7 @@ func NewHandler() *Handler { Broadcaster: NopBroadcaster, //BroadcastHandler: NopBroadcastHandler, // TODO: implement the nop //StatusHandler: NopStatusHandler, // TODO: implement the nop - StaticFileSystem: NopStaticFileSystem, + FileSystem: NopFileSystem, LogOutput: os.Stderr, } @@ -291,7 +291,7 @@ func (h *Handler) handleWebUI(w http.ResponseWriter, r *http.Request) { http.Error(w, "Welcome. Pilosa is running. Visit https://www.pilosa.com/docs/ for more information or try the WebUI by visiting this URL in your browser.", http.StatusNotFound) return } - statikFS, err := h.StaticFileSystem.New() + statikFS, err := h.FileSystem.New() if err != nil { h.writeQueryResponse(w, r, &QueryResponse{Err: err}) h.logger().Println("Pilosa WebUI is not available. Please run `make generate-statik` before building Pilosa with `make install`.") diff --git a/handler_test.go b/handler_test.go index 6386f53f9..4f3f12f77 100644 --- a/handler_test.go +++ b/handler_test.go @@ -29,9 +29,9 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa" - "github.com/pilosa/pilosa/filesystem" "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/pql" + "github.com/pilosa/pilosa/statikfs" "github.com/pilosa/pilosa/test" ) @@ -1854,7 +1854,7 @@ func TestHandler_WebUI(t *testing.T) { h := test.NewHandler() h.Holder = hldr.Holder h.Cluster = test.NewCluster(1) - h.StaticFileSystem = &filesystem.StatikFS{} + h.FileSystem = &statikfs.FileSystem{} w := httptest.NewRecorder() h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/", nil)) diff --git a/server/server.go b/server/server.go index e056bc76d..52df0e52e 100644 --- a/server/server.go +++ b/server/server.go @@ -33,9 +33,9 @@ import ( "crypto/tls" "github.com/pilosa/pilosa" - "github.com/pilosa/pilosa/filesystem" "github.com/pilosa/pilosa/gcnotify" "github.com/pilosa/pilosa/gossip" + "github.com/pilosa/pilosa/statikfs" "github.com/pilosa/pilosa/statsd" ) @@ -193,7 +193,7 @@ func (m *Command) SetupServer() error { m.Server.Cluster.RemoteClient = c // Statik file system. - m.Server.Handler.StaticFileSystem = &filesystem.StatikFS{} + m.Server.Handler.FileSystem = &statikfs.FileSystem{} // Default coordintor to port 0 when not specified so that coordinator // can be set to the value of server.URI after server binds to a port. diff --git a/filesystem/statik.go b/statikfs/filesystem.go similarity index 70% rename from filesystem/statik.go rename to statikfs/filesystem.go index 405e31a11..306034a1f 100644 --- a/filesystem/statik.go +++ b/statikfs/filesystem.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package filesystem +package statikfs import ( "net/http" @@ -21,12 +21,13 @@ import ( "github.com/rakyll/statik/fs" ) -// Ensure nopStaticFileSystem implements interface. -var _ pilosa.StaticFileSystem = &StatikFS{} +// Ensure nopFileSystem implements interface. +var _ pilosa.FileSystem = &FileSystem{} -type StatikFS struct{} +// FileSystem represents a static FileSystem. +type FileSystem struct{} -// New is a statik implementation of StaticFileSystem New method. -func (s *StatikFS) New() (http.FileSystem, error) { +// New is a statik implementation of FileSystem New method. +func (s *FileSystem) New() (http.FileSystem, error) { return fs.New() } From 0a6f2d07f6d39b31605edb3c5702fc529a93d87b Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Thu, 8 Mar 2018 08:38:29 -0600 Subject: [PATCH 3/5] Move statik filesystem implemention to subpackage of statik package. --- Makefile | 4 ++-- handler.go | 2 -- handler_test.go | 4 ++-- server/server.go | 4 ++-- statik/.gitignore | 2 +- statik/doc.go | 1 + statikfs/filesystem.go => statik/filesystem/statik.go | 2 +- 7 files changed, 9 insertions(+), 10 deletions(-) rename statikfs/filesystem.go => statik/filesystem/statik.go (98%) diff --git a/Makefile b/Makefile index b1b545042..8fdbcea3d 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: dep docker pilosa release-build prerelease-build release prerelease prerelease-upload install generate statik test cover cover-pkg cover-viz clean docker-build docker-test +.PHONY: dep docker pilosa release-build prerelease-build release prerelease prerelease-upload install generate generate-statik generate-protoc statik test cover cover-pkg cover-viz clean docker-build docker-test DEP := $(shell command -v dep 2>/dev/null) STATIK := $(shell command -v statik 2>/dev/null) @@ -103,7 +103,7 @@ generate-protoc: .protoc-gen-gofast go generate github.com/pilosa/pilosa/internal generate-statik: statik - go generate github.com/pilosa/pilosa + go generate github.com/pilosa/pilosa/statik generate: generate-protoc generate-statik diff --git a/handler.go b/handler.go index 067219c8f..e4a707d8f 100644 --- a/handler.go +++ b/handler.go @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:generate statik -src=./webui - package pilosa import ( diff --git a/handler_test.go b/handler_test.go index 4f3f12f77..5d8af43a9 100644 --- a/handler_test.go +++ b/handler_test.go @@ -31,7 +31,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/pql" - "github.com/pilosa/pilosa/statikfs" + statik "github.com/pilosa/pilosa/statik/filesystem" "github.com/pilosa/pilosa/test" ) @@ -1854,7 +1854,7 @@ func TestHandler_WebUI(t *testing.T) { h := test.NewHandler() h.Holder = hldr.Holder h.Cluster = test.NewCluster(1) - h.FileSystem = &statikfs.FileSystem{} + h.FileSystem = &statik.FileSystem{} w := httptest.NewRecorder() h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/", nil)) diff --git a/server/server.go b/server/server.go index 52df0e52e..67498d7d4 100644 --- a/server/server.go +++ b/server/server.go @@ -35,7 +35,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/gcnotify" "github.com/pilosa/pilosa/gossip" - "github.com/pilosa/pilosa/statikfs" + statik "github.com/pilosa/pilosa/statik/filesystem" "github.com/pilosa/pilosa/statsd" ) @@ -193,7 +193,7 @@ func (m *Command) SetupServer() error { m.Server.Cluster.RemoteClient = c // Statik file system. - m.Server.Handler.FileSystem = &statikfs.FileSystem{} + m.Server.Handler.FileSystem = &statik.FileSystem{} // Default coordintor to port 0 when not specified so that coordinator // can be set to the value of server.URI after server binds to a port. diff --git a/statik/.gitignore b/statik/.gitignore index 514ee40a1..485c0c57d 100644 --- a/statik/.gitignore +++ b/statik/.gitignore @@ -1 +1 @@ -statik.go +/statik.go diff --git a/statik/doc.go b/statik/doc.go index 85edd9e6f..9310eb508 100644 --- a/statik/doc.go +++ b/statik/doc.go @@ -1,3 +1,4 @@ // Package statik contains static assets for the Web UI. `go generate` will // produce statik.go, which is ignored by git. +//go:generate statik -src=../webui -dest=.. package statik diff --git a/statikfs/filesystem.go b/statik/filesystem/statik.go similarity index 98% rename from statikfs/filesystem.go rename to statik/filesystem/statik.go index 306034a1f..3f9582a71 100644 --- a/statikfs/filesystem.go +++ b/statik/filesystem/statik.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package statikfs +package filesystem import ( "net/http" From b36e8c7a1486c7944e66d1cd5ddd0f909cd7b47c Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Mon, 19 Mar 2018 08:39:17 -0500 Subject: [PATCH 4/5] Combine two subpackages and consolidate doc.go into filestystem.go for simplicity. --- handler.go | 7 ++----- handler_test.go | 2 +- server/server.go | 2 +- statik/doc.go | 4 ---- statik/{filesystem/statik.go => filesystem.go} | 7 ++++++- 5 files changed, 10 insertions(+), 12 deletions(-) delete mode 100644 statik/doc.go rename statik/{filesystem/statik.go => filesystem.go} (81%) diff --git a/handler.go b/handler.go index e4a707d8f..6e93ae380 100644 --- a/handler.go +++ b/handler.go @@ -42,9 +42,6 @@ import ( "github.com/pilosa/pilosa/pql" "unicode" - - // Allow building Pilosa without the web UI. - _ "github.com/pilosa/pilosa/statik" ) // Handler represents an HTTP handler. @@ -289,13 +286,13 @@ func (h *Handler) handleWebUI(w http.ResponseWriter, r *http.Request) { http.Error(w, "Welcome. Pilosa is running. Visit https://www.pilosa.com/docs/ for more information or try the WebUI by visiting this URL in your browser.", http.StatusNotFound) return } - statikFS, err := h.FileSystem.New() + filesystem, err := h.FileSystem.New() if err != nil { h.writeQueryResponse(w, r, &QueryResponse{Err: err}) h.logger().Println("Pilosa WebUI is not available. Please run `make generate-statik` before building Pilosa with `make install`.") return } - http.FileServer(statikFS).ServeHTTP(w, r) + http.FileServer(filesystem).ServeHTTP(w, r) } // handleGetSchema handles GET /schema requests. diff --git a/handler_test.go b/handler_test.go index 5d8af43a9..aa8ad2970 100644 --- a/handler_test.go +++ b/handler_test.go @@ -31,7 +31,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/pql" - statik "github.com/pilosa/pilosa/statik/filesystem" + "github.com/pilosa/pilosa/statik" "github.com/pilosa/pilosa/test" ) diff --git a/server/server.go b/server/server.go index 67498d7d4..b9140f6a3 100644 --- a/server/server.go +++ b/server/server.go @@ -35,7 +35,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/gcnotify" "github.com/pilosa/pilosa/gossip" - statik "github.com/pilosa/pilosa/statik/filesystem" + "github.com/pilosa/pilosa/statik" "github.com/pilosa/pilosa/statsd" ) diff --git a/statik/doc.go b/statik/doc.go deleted file mode 100644 index 9310eb508..000000000 --- a/statik/doc.go +++ /dev/null @@ -1,4 +0,0 @@ -// Package statik contains static assets for the Web UI. `go generate` will -// produce statik.go, which is ignored by git. -//go:generate statik -src=../webui -dest=.. -package statik diff --git a/statik/filesystem/statik.go b/statik/filesystem.go similarity index 81% rename from statik/filesystem/statik.go rename to statik/filesystem.go index 3f9582a71..6d7fd864f 100644 --- a/statik/filesystem/statik.go +++ b/statik/filesystem.go @@ -11,8 +11,13 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. +// +//go:generate statik -src=../webui -dest=.. +// +// Package statik contains static assets for the Web UI. `go generate` or +// `make generate-statik` will produce statik.go, which is ignored by git. -package filesystem +package statik import ( "net/http" From 68581d50c0a53c9fe82484ae55ffb477b457cee6 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Mon, 19 Mar 2018 14:04:21 -0500 Subject: [PATCH 5/5] Remove empty line between godoc and package declaration --- statik/filesystem.go | 1 - 1 file changed, 1 deletion(-) diff --git a/statik/filesystem.go b/statik/filesystem.go index 6d7fd864f..e3bf95cb1 100644 --- a/statik/filesystem.go +++ b/statik/filesystem.go @@ -16,7 +16,6 @@ // // Package statik contains static assets for the Web UI. `go generate` or // `make generate-statik` will produce statik.go, which is ignored by git. - package statik import (