Add ?fetchdata on messages collection to include data, and make consistent with get by ID

docs/swagger/swagger.yaml

@@ -2004,6 +2004,11 @@ paths:
         schema:
           example: default
           type: string
+      - description: Fetch the data and include it in the messages returned
+        in: query
+        name: fetchdata
+        schema:
+          type: string
       - description: Server-side request timeout (millseconds, or set a custom suffix
           like 10s)
         in: header
@@ -2214,11 +2219,17 @@ paths:
         required: true
         schema:
           type: string
-      - description: 'TODO: Description'
+      - deprecated: true
+        description: 'TODO: Description'
         in: query
         name: data
         schema:
           type: string
+      - description: Fetch the data and include it in the messages returned
+        in: query
+        name: fetchdata
+        schema:
+          type: string
       - description: Server-side request timeout (millseconds, or set a custom suffix
           like 10s)
         in: header

internal/apiserver/route_get_msg_by_id.go

@@ -35,15 +35,19 @@ var getMsgByID = &oapispec.Route{
 		{Name: "msgid", Description: i18n.MsgTBD},
 	},
 	QueryParams: []*oapispec.QueryParam{
-		{Name: "data", IsBool: true, Description: i18n.MsgTBD},
+		// Confusing using 'data' for this one, as we cannot use it on the collection one
+		{Name: "data", IsBool: true, Description: i18n.MsgTBD, Deprecated: true},
+		{Name: "fetchdata", IsBool: true, Description: i18n.MsgFetchDataDesc},
 	},
 	FilterFactory:   nil,
 	Description:     i18n.MsgTBD,
 	JSONInputValue:  nil,
 	JSONOutputValue: func() interface{} { return &fftypes.MessageInOut{} }, // can include full values
 	JSONOutputCodes: []int{http.StatusOK},
 	JSONHandler: func(r *oapispec.APIRequest) (output interface{}, err error) {
-		output, err = r.Or.GetMessageByID(r.Ctx, r.PP["ns"], r.PP["msgid"], strings.EqualFold(r.QP["data"], "true"))
-		return output, err
+		if strings.EqualFold(r.QP["data"], "true") || strings.EqualFold(r.QP["fetchdata"], "true") {
+			return r.Or.GetMessageByIDWithData(r.Ctx, r.PP["ns"], r.PP["msgid"])
+		}
+		return r.Or.GetMessageByID(r.Ctx, r.PP["ns"], r.PP["msgid"])
 	},
 }

internal/apiserver/route_get_msg_by_id_test.go

@@ -31,8 +31,8 @@ func TestGetMessageByID(t *testing.T) {
 	req.Header.Set("Content-Type", "application/json; charset=utf-8")
 	res := httptest.NewRecorder()
 
-	o.On("GetMessageByID", mock.Anything, "mynamespace", "abcd12345", false).
-		Return(&fftypes.MessageInOut{}, nil)
+	o.On("GetMessageByID", mock.Anything, "mynamespace", "abcd12345").
+		Return(&fftypes.Message{}, nil)
 	r.ServeHTTP(res, req)
 
 	assert.Equal(t, 200, res.Result().StatusCode)
@@ -44,7 +44,7 @@ func TestGetMessageByIDWithData(t *testing.T) {
 	req.Header.Set("Content-Type", "application/json; charset=utf-8")
 	res := httptest.NewRecorder()
 
-	o.On("GetMessageByID", mock.Anything, "mynamespace", "abcd12345", true).
+	o.On("GetMessageByIDWithData", mock.Anything, "mynamespace", "abcd12345").
 		Return(&fftypes.MessageInOut{}, nil)
 	r.ServeHTTP(res, req)
 

internal/apiserver/route_get_msgs.go

@@ -18,6 +18,7 @@ package apiserver
 
 import (
 	"net/http"
+	"strings"
 
 	"github.com/hyperledger/firefly/internal/config"
 	"github.com/hyperledger/firefly/internal/i18n"
@@ -33,13 +34,18 @@ var getMsgs = &oapispec.Route{
 	PathParams: []*oapispec.PathParam{
 		{Name: "ns", ExampleFromConf: config.NamespacesDefault, Description: i18n.MsgTBD},
 	},
-	QueryParams:     nil,
+	QueryParams: []*oapispec.QueryParam{
+		{Name: "fetchdata", IsBool: true, Description: i18n.MsgFetchDataDesc},
+	},
 	FilterFactory:   database.MessageQueryFactory,
 	Description:     i18n.MsgTBD,
 	JSONInputValue:  nil,
 	JSONOutputValue: func() interface{} { return []*fftypes.Message{} },
 	JSONOutputCodes: []int{http.StatusOK},
 	JSONHandler: func(r *oapispec.APIRequest) (output interface{}, err error) {
+		if strings.EqualFold(r.QP["fetchdata"], "true") {
+			return filterResult(r.Or.GetMessagesWithData(r.Ctx, r.PP["ns"], r.Filter))
+		}
 		return filterResult(r.Or.GetMessages(r.Ctx, r.PP["ns"], r.Filter))
 	},
 }

internal/apiserver/route_get_msgs_test.go

@@ -61,3 +61,25 @@ func TestGetMessagesWithCount(t *testing.T) {
 	assert.Equal(t, int64(0), resWithCount.Count)
 	assert.Equal(t, int64(10), resWithCount.Total)
 }
+
+func TestGetMessagesWithCountAndData(t *testing.T) {
+	o, r := newTestAPIServer()
+	req := httptest.NewRequest("GET", "/api/v1/namespaces/mynamespace/messages?count&fetchdata", nil)
+	req.Header.Set("Content-Type", "application/json; charset=utf-8")
+	res := httptest.NewRecorder()
+
+	var ten int64 = 10
+	o.On("GetMessagesWithData", mock.Anything, "mynamespace", mock.Anything).
+		Return([]*fftypes.MessageInOut{}, &database.FilterResult{
+			TotalCount: &ten,
+		}, nil)
+	r.ServeHTTP(res, req)
+
+	assert.Equal(t, 200, res.Result().StatusCode)
+	var resWithCount filterResultsWithCount
+	err := json.NewDecoder(res.Body).Decode(&resWithCount)
+	assert.NoError(t, err)
+	assert.NotNil(t, resWithCount.Items)
+	assert.Equal(t, int64(0), resWithCount.Count)
+	assert.Equal(t, int64(10), resWithCount.Total)
+}

internal/i18n/en_translations.go

@@ -206,4 +206,5 @@ var (
 	MsgFailedToDecodeCertificate   = ffm("FF10286", "Failed to decode certificate: %s", 500)
 	MsgInvalidMessageType          = ffm("FF10287", "Invalid message type - allowed types are %s", 400)
 	MsgNoUUID                      = ffm("FF10288", "Field '%s' must not be a UUID", 400)
+	MsgFetchDataDesc               = ffm("FF10289", "Fetch the data and include it in the messages returned", 400)
 )

internal/oapispec/openapi3.go

@@ -150,7 +150,7 @@ func addOutput(ctx context.Context, route *Route, output interface{}, op *openap
 	}
 }
 
-func addParam(ctx context.Context, op *openapi3.Operation, in, name, def, example string, description i18n.MessageKey, msgArgs ...interface{}) {
+func addParam(ctx context.Context, op *openapi3.Operation, in, name, def, example string, description i18n.MessageKey, deprecated bool, msgArgs ...interface{}) {
 	required := false
 	if in == "path" {
 		required = true
@@ -168,6 +168,7 @@ func addParam(ctx context.Context, op *openapi3.Operation, in, name, def, exampl
 			In:          in,
 			Name:        name,
 			Required:    required,
+			Deprecated:  deprecated,
 			Description: i18n.Expand(ctx, description, msgArgs...),
 			Schema: &openapi3.SchemaRef{
 				Value: &openapi3.Schema{
@@ -213,28 +214,28 @@ func addRoute(ctx context.Context, doc *openapi3.T, route *Route) {
 		if p.ExampleFromConf != "" {
 			example = config.GetString(p.ExampleFromConf)
 		}
-		addParam(ctx, op, "path", p.Name, p.Default, example, p.Description)
+		addParam(ctx, op, "path", p.Name, p.Default, example, p.Description, false)
 	}
 	for _, q := range route.QueryParams {
 		example := q.Example
 		if q.ExampleFromConf != "" {
 			example = config.GetString(q.ExampleFromConf)
 		}
-		addParam(ctx, op, "query", q.Name, q.Default, example, q.Description)
+		addParam(ctx, op, "query", q.Name, q.Default, example, q.Description, q.Deprecated)
 	}
-	addParam(ctx, op, "header", "Request-Timeout", config.GetString(config.APIRequestTimeout), "", i18n.MsgRequestTimeoutDesc)
+	addParam(ctx, op, "header", "Request-Timeout", config.GetString(config.APIRequestTimeout), "", i18n.MsgRequestTimeoutDesc, false)
 	if route.FilterFactory != nil {
 		fields := route.FilterFactory.NewFilter(ctx).Fields()
 		sort.Strings(fields)
 		for _, field := range fields {
-			addParam(ctx, op, "query", field, "", "", i18n.MsgFilterParamDesc)
+			addParam(ctx, op, "query", field, "", "", i18n.MsgFilterParamDesc, false)
 		}
-		addParam(ctx, op, "query", "sort", "", "", i18n.MsgFilterSortDesc)
-		addParam(ctx, op, "query", "ascending", "", "", i18n.MsgFilterAscendingDesc)
-		addParam(ctx, op, "query", "descending", "", "", i18n.MsgFilterDescendingDesc)
-		addParam(ctx, op, "query", "skip", "", "", i18n.MsgFilterSkipDesc, config.GetUint(config.APIMaxFilterSkip))
-		addParam(ctx, op, "query", "limit", "", config.GetString(config.APIDefaultFilterLimit), i18n.MsgFilterLimitDesc, config.GetUint(config.APIMaxFilterLimit))
-		addParam(ctx, op, "query", "count", "", "", i18n.MsgFilterCountDesc)
+		addParam(ctx, op, "query", "sort", "", "", i18n.MsgFilterSortDesc, false)
+		addParam(ctx, op, "query", "ascending", "", "", i18n.MsgFilterAscendingDesc, false)
+		addParam(ctx, op, "query", "descending", "", "", i18n.MsgFilterDescendingDesc, false)
+		addParam(ctx, op, "query", "skip", "", "", i18n.MsgFilterSkipDesc, false, config.GetUint(config.APIMaxFilterSkip))
+		addParam(ctx, op, "query", "limit", "", config.GetString(config.APIDefaultFilterLimit), i18n.MsgFilterLimitDesc, false, config.GetUint(config.APIMaxFilterLimit))
+		addParam(ctx, op, "query", "count", "", "", i18n.MsgFilterCountDesc, false)
 	}
 	switch route.Method {
 	case http.MethodGet:

internal/oapispec/routes.go

@@ -91,6 +91,8 @@ type QueryParam struct {
 	ExampleFromConf config.RootKey
 	// Description is a message key to a translatable description of the parameter
 	Description i18n.MessageKey
+	// Deprecated whether this param is deprecated
+	Deprecated bool
 }
 
 // FormParam is a description of a multi-part form parameter

internal/orchestrator/data_query.go

@@ -75,33 +75,31 @@ func (or *orchestrator) getMessageByID(ctx context.Context, ns, id string) (*fft
 	return msg, err
 }
 
-func (or *orchestrator) GetMessageByID(ctx context.Context, ns, id string, withValues bool) (*fftypes.MessageInOut, error) {
-	msg, err := or.getMessageByID(ctx, ns, id)
-	if err != nil {
-		return nil, err
-	}
+func (or *orchestrator) GetMessageByID(ctx context.Context, ns, id string) (*fftypes.Message, error) {
+	return or.getMessageByID(ctx, ns, id)
+}
+
+func (or *orchestrator) fetchMessageData(ctx context.Context, msg *fftypes.Message) (*fftypes.MessageInOut, error) {
 	msgI := &fftypes.MessageInOut{
 		Message: *msg,
 	}
-	if withValues {
-		// Lookup the full data
-		data, _, err := or.data.GetMessageData(ctx, msg, true)
-		if err != nil {
-			return nil, err
-		}
-		msgI.SetInlineData(data)
-	} else {
-		// Just put the data refs into the serialized struct
-		msgI.InlineData = make(fftypes.InlineData, len(msg.Data))
-		for i, dr := range msg.Data {
-			msgI.InlineData[i] = &fftypes.DataRefOrValue{
-				DataRef: *dr,
-			}
-		}
+	// Lookup the full data
+	data, _, err := or.data.GetMessageData(ctx, msg, true)
+	if err != nil {
+		return nil, err
 	}
+	msgI.SetInlineData(data)
 	return msgI, err
 }
 
+func (or *orchestrator) GetMessageByIDWithData(ctx context.Context, ns, id string) (*fftypes.MessageInOut, error) {
+	msg, err := or.getMessageByID(ctx, ns, id)
+	if err != nil {
+		return nil, err
+	}
+	return or.fetchMessageData(ctx, msg)
+}
+
 func (or *orchestrator) GetBatchByID(ctx context.Context, ns, id string) (*fftypes.Batch, error) {
 	u, err := or.verifyIDAndNamespace(ctx, ns, id)
 	if err != nil {
@@ -170,6 +168,21 @@ func (or *orchestrator) GetMessages(ctx context.Context, ns string, filter datab
 	return or.database.GetMessages(ctx, filter)
 }
 
+func (or *orchestrator) GetMessagesWithData(ctx context.Context, ns string, filter database.AndFilter) ([]*fftypes.MessageInOut, *database.FilterResult, error) {
+	filter = or.scopeNS(ns, filter)
+	msgs, fr, err := or.database.GetMessages(ctx, filter)
+	if err != nil {
+		return nil, nil, err
+	}
+	msgsData := make([]*fftypes.MessageInOut, len(msgs))
+	for i, msg := range msgs {
+		if msgsData[i], err = or.fetchMessageData(ctx, msg); err != nil {
+			return nil, nil, err
+		}
+	}
+	return msgsData, fr, err
+}
+
 func (or *orchestrator) GetMessageData(ctx context.Context, ns, id string) ([]*fftypes.Data, error) {
 	msg, err := or.getMessageByID(ctx, ns, id)
 	if err != nil || msg == nil {