Merge pull request #185 from hyperledger/filter-ext

Add ability to dynamically add filter JSON fields (and test coverage)

Author: EnriqueL8

pkg/ffapi/apiserver.go

@@ -202,7 +202,7 @@ func (as *apiServer[T]) Serve(ctx context.Context) (err error) {
 		if err != nil {
 			return err
 		}
-		as.monitoringPublicURL = buildPublicURL(as.MonitoringConfig, apiHTTPServer.Addr())
+		as.monitoringPublicURL = buildPublicURL(as.MonitoringConfig, monitoringHTTPServer.Addr())
 		go monitoringHTTPServer.ServeHTTP(ctx)
 	}
 
@@ -219,6 +219,10 @@ func (as *apiServer[T]) APIPublicURL() string {
 	return as.apiPublicURL
 }
 
+func (as *apiServer[T]) MonitoringPublicURL() string {
+	return as.monitoringPublicURL
+}
+
 func (as *apiServer[T]) waitForServerStop(httpErrChan, monitoringErrChan chan error) error {
 	select {
 	case err := <-httpErrChan:
@@ -382,9 +386,8 @@ func (as *apiServer[T]) notFoundHandler(res http.ResponseWriter, req *http.Reque
 	return 404, i18n.NewError(req.Context(), i18n.Msg404NotFound)
 }
 
-func (as *apiServer[T]) emptyJSONHandler(res http.ResponseWriter, _ *http.Request) (status int, err error) {
-	res.Header().Add("Content-Type", "application/json")
-	return 200, nil
+func (as *apiServer[T]) noContentResponder(res http.ResponseWriter, _ *http.Request) {
+	res.WriteHeader(http.StatusNoContent)
 }
 
 func (as *apiServer[T]) createMonitoringMuxRouter(ctx context.Context) (*mux.Router, error) {
@@ -396,7 +399,7 @@ func (as *apiServer[T]) createMonitoringMuxRouter(ctx context.Context) (*mux.Rou
 		panic(err)
 	}
 	r.Path(as.metricsPath).Handler(h)
-	r.HandleFunc(as.livenessPath, hf.APIWrapper(as.emptyJSONHandler))
+	r.HandleFunc(as.livenessPath, as.noContentResponder)
 
 	for _, route := range as.MonitoringRoutes {
 		path := route.Path

pkg/ffapi/apiserver_test.go

@@ -18,6 +18,7 @@ package ffapi
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
@@ -112,6 +113,29 @@ var utAPIRoute2 = &Route{
 	},
 }
 
+type testInputStruct struct {
+	Input1 string `json:"input1,omitempty"`
+}
+
+var utAPIRoute3 = &Route{
+	Name:        "utAPIRoute3",
+	Path:        "ut/utresource/{resourceid}/postbatch",
+	Method:      http.MethodPost,
+	Description: "post an array to check arrays go through ok",
+	JSONInputDecoder: func(req *http.Request, body io.Reader) (interface{}, error) {
+		var arrayInput []*testInputStruct
+		err := json.NewDecoder(body).Decode(&arrayInput)
+		return arrayInput, err
+	},
+	JSONInputValue:  func() interface{} { return []*testInputStruct{} },
+	JSONOutputValue: func() interface{} { return []*testInputStruct{} },
+	Extensions: &APIServerRouteExt[*utManager]{
+		JSONHandler: func(a *APIRequest, um *utManager) (output interface{}, err error) {
+			return a.Input.([]*testInputStruct), nil
+		},
+	},
+}
+
 func initUTConfig() (config.Section, config.Section, config.Section) {
 	config.RootConfigReset()
 	apiConfig := config.RootSection("ut.api")
@@ -129,7 +153,7 @@ func newTestAPIServer(t *testing.T, start bool) (*utManager, *apiServer[*utManag
 	um := &utManager{t: t}
 	as := NewAPIServer(ctx, APIServerOptions[*utManager]{
 		MetricsRegistry: metric.NewPrometheusMetricsRegistry("ut"),
-		Routes:          []*Route{utAPIRoute1, utAPIRoute2},
+		Routes:          []*Route{utAPIRoute1, utAPIRoute2, utAPIRoute3},
 		EnrichRequest: func(r *APIRequest) (*utManager, error) {
 			// This could be some dynamic object based on extra processing in the request,
 			// but the most common case is you just have a "manager" that you inject into each
@@ -176,6 +200,50 @@ func TestAPIServerInvokeAPIRouteStream(t *testing.T) {
 	assert.Equal(t, "a stream!", string(res.Body()))
 }
 
+func TestAPIServerInvokeAPIPostEmptyArray(t *testing.T) {
+	_, as, done := newTestAPIServer(t, true)
+	defer done()
+
+	<-as.Started()
+
+	var o []*testInputStruct
+	res, err := resty.New().R().
+		SetBody([]*testInputStruct{}).
+		SetResult(&o).
+		Post(fmt.Sprintf("%s/api/v1/ut/utresource/id12345/postbatch", as.APIPublicURL()))
+	assert.NoError(t, err)
+	assert.Equal(t, 200, res.StatusCode())
+	assert.Equal(t, []*testInputStruct{}, o)
+
+	res, err = resty.New().R().
+		SetBody([]*testInputStruct{{Input1: "in1"}}).
+		SetResult(&o).
+		Post(fmt.Sprintf("%s/api/v1/ut/utresource/id12345/postbatch", as.APIPublicURL()))
+	assert.NoError(t, err)
+	assert.Equal(t, 200, res.StatusCode())
+	assert.Equal(t, []*testInputStruct{{Input1: "in1"}}, o)
+}
+
+func TestAPIServerInvokeAPIRouteLiveness(t *testing.T) {
+	_, as, done := newTestAPIServer(t, true)
+	defer done()
+
+	<-as.Started()
+
+	res, err := resty.New().R().Get(fmt.Sprintf("%s/livez", as.MonitoringPublicURL()))
+	assert.NoError(t, err)
+	assert.Equal(t, 204, res.StatusCode())
+}
+
+func TestAPIServerPanicsMisConfig(t *testing.T) {
+	assert.Panics(t, func() {
+		_ = NewAPIServer(context.Background(), APIServerOptions[any]{})
+	})
+	assert.Panics(t, func() {
+		_ = NewAPIServer(context.Background(), APIServerOptions[any]{APIConfig: config.RootSection("any")})
+	})
+}
+
 func TestAPIServerInvokeAPIRouteJSON(t *testing.T) {
 	um, as, done := newTestAPIServer(t, true)
 	defer done()
@@ -342,6 +410,19 @@ func TestAPIServerInvokeEnrichFailForm(t *testing.T) {
 	assert.Equal(t, 500, res.StatusCode())
 }
 
+func TestAPIServerInvokeEnrichFailStream(t *testing.T) {
+	um, as, done := newTestAPIServer(t, true)
+	defer done()
+
+	um.mockEnrichErr = fmt.Errorf("pop")
+	<-as.Started()
+
+	res, err := resty.New().R().
+		Get(fmt.Sprintf("%s/api/v1/ut/utresource/id12345/getit", as.APIPublicURL()))
+	assert.NoError(t, err)
+	assert.Equal(t, 500, res.StatusCode())
+}
+
 func TestAPIServer404(t *testing.T) {
 	_, as, done := newTestAPIServer(t, true)
 	defer done()
@@ -397,6 +478,45 @@ func TestAPIServerFailServeMonitoring(t *testing.T) {
 
 }
 
+func TestAPIServerFailServeMonitoringBadRouteSlash(t *testing.T) {
+	_, as, done := newTestAPIServer(t, false)
+	defer done()
+
+	as.MonitoringConfig.Set(httpserver.HTTPConfAddress, "127.0.0.1:0")
+	as.MonitoringRoutes = []*Route{
+		{Path: "right", Extensions: &APIServerRouteExt[*utManager]{}},
+		{Path: "/wrong"},
+	}
+	err := as.Serve(context.Background())
+	assert.Regexp(t, "FF00255", err)
+
+	// Check we still closed the started channel
+	<-as.Started()
+
+}
+
+func TestAPIServerFailServeBadRouteSlash(t *testing.T) {
+	_, as, done := newTestAPIServer(t, false)
+	defer done()
+
+	as.Routes = []*Route{
+		{
+			Path: "/wrong",
+			Extensions: &APIServerRouteExt[*utManager]{
+				JSONHandler: func(a *APIRequest, um *utManager) (output interface{}, err error) {
+					return nil, nil
+				},
+			},
+		},
+	}
+	err := as.Serve(context.Background())
+	assert.Regexp(t, "FF00255", err)
+
+	// Check we still closed the started channel
+	<-as.Started()
+
+}
+
 func TestWaitForServerStop(t *testing.T) {
 	_, as, done := newTestAPIServer(t, false)
 	defer done()
@@ -611,6 +731,7 @@ func TestVersionedAPIInitErrors(t *testing.T) {
 	err = as.Serve(ctx)
 	assert.Error(t, err)
 	assert.Regexp(t, "FF00253", err)
+
 	as = NewAPIServer(ctx, APIServerOptions[*utManager]{
 		MetricsRegistry: metric.NewPrometheusMetricsRegistry("ut"),
 		VersionedAPIs: &VersionedAPIs{
@@ -633,4 +754,32 @@ func TestVersionedAPIInitErrors(t *testing.T) {
 	assert.Error(t, err)
 	assert.Regexp(t, "FF00254", err)
 
+	as = NewAPIServer(ctx, APIServerOptions[*utManager]{
+		MetricsRegistry: metric.NewPrometheusMetricsRegistry("ut"),
+		VersionedAPIs: &VersionedAPIs{
+			DefaultVersion: "unknown",
+			APIVersions: map[string]*APIVersion{
+				"v1": {
+					Routes: []*Route{
+						{
+							Path: "/wrong",
+							Extensions: &APIServerRouteExt[*utManager]{
+								JSONHandler: func(r *APIRequest, um *utManager) (output interface{}, err error) {
+									return nil, nil
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		Description:      "unit testing",
+		APIConfig:        apiConfig,
+		MonitoringConfig: monitoringConfig,
+	})
+
+	err = as.Serve(ctx)
+	assert.Error(t, err)
+	assert.Regexp(t, "FF00255", err)
+
 }

pkg/ffapi/handler.go

@@ -216,7 +216,9 @@ func (hs *HandlerFactory) RouteHandler(route *Route) http.HandlerFunc {
 				req.Header.Set("Content-Type", "application/json; charset=utf8")
 				fallthrough
 			case strings.HasPrefix(strings.ToLower(contentType), "application/json"):
-				if jsonInput != nil {
+				if route.JSONInputDecoder != nil {
+					jsonInput, err = route.JSONInputDecoder(req, req.Body)
+				} else if jsonInput != nil {
 					d := json.NewDecoder(req.Body)
 					d.UseNumber()
 					err = d.Decode(&jsonInput)

pkg/ffapi/handler_test.go

@@ -28,6 +28,7 @@ import (
 	"net/url"
 	"strings"
 	"testing"
+	"testing/iotest"
 	"time"
 
 	"github.com/getkin/kin-openapi/openapi3"
@@ -727,3 +728,23 @@ func TestPOSTFormParamsMultiValueUnsupported(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, 400, res.StatusCode)
 }
+
+func TestGetFormParamsFail(t *testing.T) {
+
+	hs := newTestHandlerFactory("", nil)
+	req := httptest.NewRequest(http.MethodPost, "http://localhost:12345/anything", iotest.ErrReader(fmt.Errorf("pop")))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	_, err := hs.getFormParams(req)
+	require.Regexp(t, "FF00250.*pop", err)
+}
+
+func TestGetFormEmptyValue(t *testing.T) {
+
+	hs := newTestHandlerFactory("", nil)
+	req := httptest.NewRequest(http.MethodPost, "http://localhost:12345/anything", nil)
+	req.Form = url.Values{
+		"nothing": []string{},
+	}
+	_, err := hs.getFormParams(req)
+	require.NoError(t, err)
+}

pkg/ffapi/openapi3_test.go

@@ -178,6 +178,21 @@ var testRoutes = []*Route{
 		JSONOutputValue: func() interface{} { return &TestStruct1{} },
 		JSONOutputCodes: []int{http.StatusOK},
 	},
+	{
+		Name:            "op7",
+		Path:            "example7",
+		Method:          http.MethodPut,
+		PathParams:      nil,
+		QueryParams:     nil,
+		Description:     ExampleDesc,
+		JSONInputValue:  func() interface{} { return &TestStruct1{} },
+		JSONOutputValue: func() interface{} { return nil },
+		JSONOutputCodes: []int{http.StatusNoContent},
+		FormParams: []*FormParam{
+			{Name: "metadata", Description: ExampleDesc},
+		},
+		Tag: example2TagName,
+	},
 }
 
 type TestInOutType struct {

pkg/ffapi/openapihandler.go

@@ -95,10 +95,6 @@ func (ohf *OpenAPIHandlerFactory) getPublicURL(req *http.Request, relativePath s
 	if publicURL == "" {
 		publicURL = ohf.StaticPublicURL
 	}
-	if publicURL == "" {
-		// Do not recommend this fallback - StaticPublicURL and/or DynamicPublicURLHeader should be set
-		publicURL = fmt.Sprintf("https://%s", req.Host)
-	}
 	publicURL = strings.TrimSuffix(publicURL, "/")
 	if len(relativePath) > 0 {
 		publicURL = publicURL + "/" + strings.TrimPrefix(relativePath, "/")

pkg/ffapi/openapihandler_test.go

@@ -24,7 +24,9 @@ import (
 	"testing"
 
 	"github.com/go-resty/resty/v2"
+	"github.com/gorilla/mux"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestOpenAPI3SwaggerUI(t *testing.T) {
@@ -98,3 +100,28 @@ func TestOpenAPI3SwaggerUIDynamicPublicURL(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Contains(t, string(b), "https://example.host.com:12345/api/myswagger.json")
 }
+
+func TestOpenAPIHandlerNonVersioned(t *testing.T) {
+	mux := mux.NewRouter()
+	hf := HandlerFactory{}
+	oah := &OpenAPIHandlerFactory{
+		BaseSwaggerGenOptions: SwaggerGenOptions{
+			Title:                 "FireFly Transaction Manager API",
+			Version:               "1.0",
+			SupportFieldRedaction: true,
+		},
+	}
+	mux.Path("/api/spec.json").Methods(http.MethodGet).Handler(hf.APIWrapper(oah.OpenAPIHandler("", OpenAPIFormatJSON, []*Route{})))
+	mux.Path("/api/spec.yaml").Methods(http.MethodGet).Handler(hf.APIWrapper(oah.OpenAPIHandler("", OpenAPIFormatYAML, []*Route{})))
+
+	ts := httptest.NewServer(mux)
+	defer ts.Close()
+
+	res, err := resty.New().R().Get(fmt.Sprintf("%s/api/spec.json", ts.URL))
+	require.NoError(t, err)
+	require.True(t, res.IsSuccess())
+
+	res, err = resty.New().R().Get(fmt.Sprintf("%s/api/spec.yaml", ts.URL))
+	require.NoError(t, err)
+	require.True(t, res.IsSuccess())
+}