refactor(lib, api, cmd): adjust log methods to use dispatch pattern

Merge pull request #1730 from qri-io/ramfox/refactor_log_dispatch

api/api.go

@@ -291,13 +291,10 @@ func NewServerRoutes(s Server) *mux.Router {
 	routeParams = newrefRouteParams(lib.AERender, false, false, http.MethodGet, http.MethodPost)
 	handleRefRoute(m, routeParams, s.Middleware(renderh.RenderHandler))
 
-	lh := NewLogHandlers(s.Instance)
-	routeParams = newrefRouteParams(lib.AEHistory, false, false, http.MethodGet, http.MethodPost)
-	handleRefRoute(m, routeParams, s.Middleware(lh.LogHandler))
-	routeParams = newrefRouteParams(lib.AELogbook, false, false, http.MethodGet, http.MethodPost)
-	handleRefRoute(m, routeParams, s.Middleware(lh.LogbookHandler))
-	m.Handle(lib.AELogs.String(), s.Middleware(lh.PlainLogsHandler))
-	m.Handle(lib.AELogbookSummary.String(), s.Middleware(lh.LogbookSummaryHandler))
+	m.Handle(lib.AEHistory.String(), s.Middleware(lib.NewHTTPRequestHandler(s.Instance, "log.history"))).Methods(http.MethodPost)
+	m.Handle(lib.AEEntries.String(), s.Middleware(lib.NewHTTPRequestHandler(s.Instance, "log.entries"))).Methods(http.MethodPost)
+	m.Handle(lib.AERawLogbook.String(), s.Middleware(lib.NewHTTPRequestHandler(s.Instance, "log.rawlogbook"))).Methods(http.MethodPost)
+	m.Handle(lib.AELogbookSummary.String(), s.Middleware(lib.NewHTTPRequestHandler(s.Instance, "log.logbooksummary"))).Methods(http.MethodPost)
 
 	rch := NewRegistryClientHandlers(s.Instance, cfg.API.ReadOnly)
 	m.Handle(lib.AERegistryNew.String(), s.Middleware(rch.CreateProfileHandler))

api/api_test.go

@@ -255,7 +255,6 @@ func TestServerReadOnlyRoutes(t *testing.T) {
 		{"GET", "/profile/photo?peername=me", 200},
 		{"GET", "/profile/poster?peername=me", 200},
 		{"GET", "/get/peer/movies", 200},
-		{"GET", "/history/peer/movies", 200},
 	}
 
 	for i, c := range cases {

api/fsi_test.go

@@ -188,7 +188,7 @@ func TestNoHistory(t *testing.T) {
 		t.Errorf("api response (-want +got):\n%s", diff)
 	}
 
-	logHandler := NewLogHandlers(run.Inst)
+	historyHandler := lib.NewHTTPRequestHandler(run.Inst, "log.history")
 
 	expectNoHistoryBody := `{
   "meta": {
@@ -198,16 +198,18 @@ func TestNoHistory(t *testing.T) {
 }`
 
 	// History with no history
-	gotStatusCode, gotBodyString = APICall("/history/peer/test_ds", logHandler.LogHandler, map[string]string{"peername": "peer", "name": "test_ds"})
+	p := lib.HistoryParams{Ref: "peer/test_ds"}
+	gotStatusCode, gotBodyString = JSONAPICallWithBody("POST", lib.AEHistory.String(), p, historyHandler, nil)
 	if gotStatusCode != 422 {
 		t.Errorf("expected status code 422, got %d", gotStatusCode)
 	}
 	if diff := cmp.Diff(expectNoHistoryBody, gotBodyString); diff != "" {
 		t.Errorf("expected body %v, got %v\ndiff:%v", expectNoHistoryBody, gotBodyString, diff)
 	}
 
+	p.EnsureFSIExists = true
 	// History with no history, still returns ErrNoHistory since this route ignores fsi param
-	gotStatusCode, gotBodyString = APICall("/history/peer/test_ds?fsi=true", logHandler.LogHandler, map[string]string{"peername": "peer", "name": "test_ds"})
+	gotStatusCode, gotBodyString = JSONAPICallWithBody("POST", lib.AEHistory.String(), p, historyHandler, nil)
 	if gotStatusCode != 422 {
 		t.Errorf("expected status code 422, got %d", gotStatusCode)
 	}

api/remote_test.go

@@ -4,6 +4,8 @@ import (
 	// "math/rand"
 	"context"
 	"testing"
+
+	"github.com/qri-io/qri/lib"
 )
 
 func TestRemoteClientHandlers(t *testing.T) {
@@ -16,7 +18,7 @@ func TestRemoteClientHandlers(t *testing.T) {
 	defer cancel()
 
 	inst := newTestInstanceWithProfileFromNode(ctx, node)
-	l := NewLogHandlers(inst)
+	historyHandler := lib.NewHTTPRequestHandler(inst, "log.history")
 	h := NewRemoteClientHandlers(inst, false)
 
 	publishCases := []handlerTestCase{
@@ -28,8 +30,8 @@ func TestRemoteClientHandlers(t *testing.T) {
 
 	// tests getting a list of logs from a remote
 	fetchCases := []handlerTestCase{
-		{"GET", "/history/", nil, nil},
-		{"GET", "/history/me/cities", nil, nil},
+		{"POST", "/history/", nil, nil},
+		{"POST", "/history/me/cities", nil, nil},
 	}
-	runHandlerTestCases(t, "fetch", l.LogHandler, fetchCases, true)
+	runHandlerTestCases(t, "fetch", historyHandler, fetchCases, true)
 }

api/testdata/api.snapshot

@@ -5062,12 +5062,12 @@ HTTP/1.1 200 OK
 Connection: close
 
 /* snapshot: TestFSIHandlers init case 0: POST / */
-HTTP/1.1 500 Internal Server Error
+HTTP/1.1 400 Bad Request
 Connection: close
 
 {
   "meta": {
-    "code": 500,
+    "code": 400,
     "error": "dataset name must start with a lower-case letter, and only contain lower-case letters, numbers, dashes, and underscore. Maximum length is 144 characters"
   }
 }

cmd/factory.go

@@ -27,7 +27,6 @@ type Factory interface {
 
 	RemoteMethods() (*lib.RemoteMethods, error)
 	RegistryClientMethods() (*lib.RegistryClientMethods, error)
-	LogMethods() (*lib.LogMethods, error)
 	ProfileMethods() (*lib.ProfileMethods, error)
 	SearchMethods() (*lib.SearchMethods, error)
 	RenderMethods() (*lib.RenderMethods, error)

cmd/factory_test.go

@@ -142,11 +142,6 @@ func (t TestFactory) RegistryClientMethods() (*lib.RegistryClientMethods, error)
 	return lib.NewRegistryClientMethods(t.inst), nil
 }
 
-// LogMethods generates a lib.LogMethods from internal state
-func (t TestFactory) LogMethods() (*lib.LogMethods, error) {
-	return lib.NewLogMethods(t.inst), nil
-}
-
 // ProfileMethods generates a lib.ProfileMethods from internal state
 func (t TestFactory) ProfileMethods() (*lib.ProfileMethods, error) {
 	return lib.NewProfileMethods(t.inst), nil

cmd/fetch_test.go

@@ -151,21 +151,21 @@ func TestFetchCommand(t *testing.T) {
 		lib.OptStdIOStreams(),
 		lib.OptSetIPFSPath(b.RepoRoot.IPFSPath),
 	)
-	localLogHandler := api.NewLogHandlers(localInst)
+	localLogHandler := lib.NewHTTPRequestHandler(localInst, "log.history")
 
 	//
 	// Validate the outputs of history and fetch
 	//
 
-	remoteLogHandler := api.NewLogHandlers(remoteServer.Instance)
+	remoteLogHandler := lib.NewHTTPRequestHandler(remoteServer.Instance, "log.history")
 
 	// Validates output of history for a remote dataset getting history for a
 	// dataset in its own namespace in its own repo
 	actualStatusCode, actualBody := APICall(
-		"GET",
-		"/history/peer_a/test_movies",
-		nil,
-		remoteLogHandler.LogHandler)
+		"POST",
+		"/history",
+		map[string]string{"refstr": "peer_a/test_movies"},
+		remoteLogHandler)
 	if actualStatusCode != 200 {
 		t.Errorf("expected status code 200, got %d", actualStatusCode)
 	}
@@ -177,12 +177,13 @@ func TestFetchCommand(t *testing.T) {
 
 	// Validates output of fetching from a remote for a remote dataset
 	actualStatusCode, actualBody = APICall(
-		"GET",
-		"/history/peer_a/test_movies",
+		"POST",
+		"/history",
 		map[string]string{
+			"refstr": "peer_a/test_movies",
 			"remote": "a_node",
 		},
-		localLogHandler.LogHandler)
+		localLogHandler)
 	if actualStatusCode != 200 {
 		t.Errorf("expected status code 200, got %d", actualStatusCode)
 	}