validation: add more validation; move testing to geth/params module

cmd/statusd/library.go

@@ -6,6 +6,8 @@ import (
 	"fmt"
 	"os"
 
+	"gopkg.in/go-playground/validator.v9"
+
 	"github.com/status-im/status-go/geth/common"
 	"github.com/status-im/status-go/geth/params"
 )
@@ -44,8 +46,44 @@ func StopNode() *C.char {
 
 //export ValidateNodeConfig
 func ValidateNodeConfig(configJSON *C.char) *C.char {
+	var resp common.APIDetailedResponse
+
 	_, err := params.LoadNodeConfig(C.GoString(configJSON))
-	return makeJSONResponse(err)
+
+	// Convert errors to common.APIDetailedResponse
+	switch err := err.(type) {
+	case validator.ValidationErrors:
+		resp = common.APIDetailedResponse{
+			Message:     "validation: validation failed",
+			FieldErrors: make([]common.APIFieldError, len(err)),
+		}
+
+		for i, ve := range err {
+			resp.FieldErrors[i] = common.APIFieldError{
+				Parameter: ve.Namespace(),
+				Errors: []common.APIError{
+					{
+						Message: fmt.Sprintf("field validation failed on the '%s' tag", ve.Tag()),
+					},
+				},
+			}
+		}
+	case error:
+		resp = common.APIDetailedResponse{
+			Message: fmt.Sprintf("validation: %s", err.Error()),
+		}
+	case nil:
+		resp = common.APIDetailedResponse{
+			Status: true,
+		}
+	}
+
+	respJSON, err := json.Marshal(resp)
+	if err != nil {
+		return makeJSONResponse(err)
+	}
+
+	return C.CString(string(respJSON))
 }
 
 //export ResetChainData

cmd/statusd/library_test.go

@@ -2,6 +2,9 @@ package main
 
 import (
 	"testing"
+
+	"github.com/status-im/status-go/geth/common"
+	"github.com/stretchr/testify/require"
 )
 
 // the actual test functions are in non-_test.go files (so that they can use cgo i.e. import "C")
@@ -12,3 +15,58 @@ func TestExportedAPI(t *testing.T) {
 
 	<-allTestsDone
 }
+
+func TestValidateNodeConfig(t *testing.T) {
+	noErrorsCallback := func(resp common.APIDetailedResponse) {
+		require.True(t, resp.Status, "expected status equal true")
+		require.Empty(t, resp.FieldErrors)
+		require.Empty(t, resp.Message)
+	}
+
+	testCases := []struct {
+		Name     string
+		Config   string
+		Callback func(common.APIDetailedResponse)
+	}{
+		{
+			Name: "response for valid config",
+			Config: `{
+				"NetworkId": 1,
+				"DataDir": "/tmp"
+			}`,
+			Callback: noErrorsCallback,
+		},
+		{
+			Name:   "response for invalid JSON string",
+			Config: `{"Network": }`,
+			Callback: func(resp common.APIDetailedResponse) {
+				require.False(t, resp.Status)
+				require.Contains(t, resp.Message, "validation: invalid character '}'")
+			},
+		},
+		{
+			Name:   "response for config with multiple errors",
+			Config: `{}`,
+			Callback: func(resp common.APIDetailedResponse) {
+				required := map[string]string{
+					"NodeConfig.NetworkID": "required",
+					"NodeConfig.DataDir":   "required",
+				}
+
+				require.False(t, resp.Status)
+				require.Contains(t, resp.Message, "validation: validation failed")
+				require.Equal(t, 2, len(resp.FieldErrors))
+
+				for _, err := range resp.FieldErrors {
+					require.Contains(t, required, err.Parameter)
+					require.Contains(t, err.Error(), required[err.Parameter])
+				}
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Logf("TestValidateNodeConfig: %s", tc.Name)
+		testValidateNodeConfig(t, tc.Config, tc.Callback)
+	}
+}

cmd/statusd/utils.go

@@ -23,6 +23,7 @@ import (
 	"github.com/status-im/status-go/geth/params"
 	. "github.com/status-im/status-go/geth/testing"
 	"github.com/status-im/status-go/static"
+	"github.com/stretchr/testify/require"
 )
 
 const zeroHash = "0x0000000000000000000000000000000000000000000000000000000000000000"
@@ -56,10 +57,6 @@ func testExportedAPI(t *testing.T, done chan struct{}) {
 			"stop/resume node",
 			testStopResumeNode,
 		},
-		{
-			"validate node config",
-			testValidateNodeConfig,
-		},
 		{
 			"call RPC on in-proc handler",
 			testCallRPC,
@@ -388,23 +385,6 @@ func testStopResumeNode(t *testing.T) bool {
 	return true
 }
 
-func testValidateNodeConfig(t *testing.T) bool {
-	result := ValidateNodeConfig(C.CString(nodeConfigJSON))
-
-	var response common.APIResponse
-	if err := json.Unmarshal([]byte(C.GoString(result)), &response); err != nil {
-		t.Errorf("could not unmarshal response as common.APIResponse: %s", err)
-		return false
-	}
-
-	if response.Error != "" {
-		t.Errorf("expected valid configuration but error occured: %s", response.Error)
-		return false
-	}
-
-	return true
-}
-
 func testCallRPC(t *testing.T) bool {
 	expected := `{"jsonrpc":"2.0","id":64,"result":"0x47173285a8d7341e5e972fc677286384f802f8ef42a5ec5f03bbfa254cb01fad"}` + "\n"
 	rawResponse := CallRPC(C.CString(`{"jsonrpc":"2.0","method":"web3_sha3","params":["0x68656c6c6f20776f726c64"],"id":64}`))
@@ -1497,3 +1477,14 @@ func startTestNode(t *testing.T) <-chan struct{} {
 
 	return waitForNodeStart
 }
+
+func testValidateNodeConfig(t *testing.T, config string, fn func(common.APIDetailedResponse)) {
+	result := ValidateNodeConfig(C.CString(config))
+
+	var resp common.APIDetailedResponse
+
+	err := json.Unmarshal([]byte(C.GoString(result)), &resp)
+	require.NoError(t, err)
+
+	fn(resp)
+}

geth/common/types.go

@@ -1,8 +1,11 @@
 package common
 
 import (
+	"bytes"
 	"encoding/json"
 	"errors"
+	"fmt"
+	"strings"
 	"time"
 
 	"github.com/ethereum/go-ethereum/accounts"
@@ -204,6 +207,56 @@ type APIResponse struct {
 	Error string `json:"error"`
 }
 
+// APIDetailedResponse represents a generic response
+// with possible errors.
+type APIDetailedResponse struct {
+	Status      bool            `json:"status"`
+	Message     string          `json:"message,omitempty"`
+	FieldErrors []APIFieldError `json:"field_errors,omitempty"`
+}
+
+func (r APIDetailedResponse) Error() string {
+	buf := bytes.NewBufferString("")
+
+	for _, err := range r.FieldErrors {
+		buf.WriteString(err.Error())
+		buf.WriteString("\n")
+	}
+
+	return strings.TrimSpace(buf.String())
+}
+
+// APIFieldError represents a set of errors
+// related to a parameter.
+type APIFieldError struct {
+	Parameter string     `json:"parameter,omitempty"`
+	Errors    []APIError `json:"errors"`
+}
+
+func (e APIFieldError) Error() string {
+	if len(e.Errors) == 0 {
+		return ""
+	}
+
+	buf := bytes.NewBufferString(fmt.Sprintf("Parameter: %s\n", e.Parameter))
+
+	for _, err := range e.Errors {
+		buf.WriteString(err.Error())
+		buf.WriteString("\n")
+	}
+
+	return strings.TrimSpace(buf.String())
+}
+
+// APIError represents a single error.
+type APIError struct {
+	Message string `json:"message"`
+}
+
+func (e APIError) Error() string {
+	return fmt.Sprintf("message=%s", e.Message)
+}
+
 // AccountInfo represents account's info
 type AccountInfo struct {
 	Address  string `json:"address"`

geth/params/config.go

@@ -215,7 +215,7 @@ type NodeConfig struct {
 	DevMode bool
 
 	// NetworkID sets network to use for selecting peers to connect to
-	NetworkID uint64 `json:"NetworkId," validate:"required,network"`
+	NetworkID uint64 `json:"NetworkId" validate:"required"`
 
 	// DataDir is the file system folder the node should use for any data storage needs.
 	DataDir string `validate:"required"`
@@ -282,22 +282,22 @@ type NodeConfig struct {
 	LogFile string
 
 	// LogLevel defines minimum log level. Valid names are "ERROR", "WARNING", "INFO", "DEBUG", and "TRACE".
-	LogLevel string
+	LogLevel string `validate:"eq=ERROR|eq=WARNING|eq=INFO|eq=DEBUG|eq=TRACE"`
 
 	// LogToStderr defines whether logged info should also be output to os.Stderr
 	LogToStderr bool
 
 	// BootClusterConfig extra configuration for supporting cluster
-	BootClusterConfig *BootClusterConfig `json:"BootClusterConfig,"`
+	BootClusterConfig *BootClusterConfig `json:"BootClusterConfig," validate:"structonly"`
 
 	// LightEthConfig extra configuration for LES
-	LightEthConfig *LightEthConfig `json:"LightEthConfig,"`
+	LightEthConfig *LightEthConfig `json:"LightEthConfig," validate:"structonly"`
 
 	// WhisperConfig extra configuration for SHH
-	WhisperConfig *WhisperConfig `json:"WhisperConfig,"`
+	WhisperConfig *WhisperConfig `json:"WhisperConfig," validate:"structonly"`
 
 	// SwarmConfig extra configuration for Swarm and ENS
-	SwarmConfig *SwarmConfig `json:"SwarmConfig,"`
+	SwarmConfig *SwarmConfig `json:"SwarmConfig," validate:"structonly"`
 }
 
 // NewNodeConfig creates new node configuration object
@@ -400,7 +400,36 @@ func loadNodeConfig(configJSON string) (*NodeConfig, error) {
 //
 func (c *NodeConfig) Validate() error {
 	validate := NewValidator()
-	return validate.Struct(c)
+
+	if err := validate.Struct(c); err != nil {
+		return err
+	}
+
+	if c.BootClusterConfig.Enabled {
+		if err := validate.Struct(c.BootClusterConfig); err != nil {
+			return err
+		}
+	}
+
+	if c.LightEthConfig.Enabled {
+		if err := validate.Struct(c.LightEthConfig); err != nil {
+			return err
+		}
+	}
+
+	if c.WhisperConfig.Enabled {
+		if err := validate.Struct(c.WhisperConfig); err != nil {
+			return err
+		}
+	}
+
+	if c.SwarmConfig.Enabled {
+		if err := validate.Struct(c.SwarmConfig); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }
 
 // Save dumps configuration to the disk