diff --git a/fileTransfer/connect/listener.go b/fileTransfer/connect/listener.go index 12b64eb0f5f125cefaaa9b5dbda9ff52018e66ec..94627f5fb6180d4d8cd792bd2f7d79d595b041cf 100644 --- a/fileTransfer/connect/listener.go +++ b/fileTransfer/connect/listener.go @@ -21,8 +21,8 @@ const ( // Name of listener (used for debugging) const listenerName = "NewFileTransferListener-Connection" -// listener waits for a message indicating a new file transfer is starting. -// Adheres to the receive.Listener interface. +// listener waits for a message indicating a new file transfer is starting. This +// structure adheres to the [receive.Listener] interface. type listener struct { m *Wrapper } diff --git a/fileTransfer/connect/params.go b/fileTransfer/connect/params.go index 5382603ffc0c486b6b6e4951ee6d6fdb43781c24..4acca2443ac0437f69406d966393af4724ba5cef 100644 --- a/fileTransfer/connect/params.go +++ b/fileTransfer/connect/params.go @@ -19,11 +19,7 @@ const ( type Params struct { // NotifyUponCompletion indicates if a final notification message is sent // to the recipient on completion of file transfer. If true, the ping is - NotifyUponCompletion bool -} - -// paramsDisk will be the marshal-able and unmarshalable object. -type paramsDisk struct { + // sent. NotifyUponCompletion bool } @@ -34,8 +30,10 @@ func DefaultParams() Params { } } -// GetParameters returns the default Params, or override with given parameters, -// if set. +// GetParameters returns the default network parameters, or override with given +// parameters, if set. Returns an error if provided invalid JSON. If the JSON is +// valid but does not match the Params structure, the default parameters will be +// returned. func GetParameters(params string) (Params, error) { p := DefaultParams() if len(params) > 0 { @@ -46,23 +44,3 @@ func GetParameters(params string) (Params, error) { } return p, nil } - -// MarshalJSON adheres to the json.Marshaler interface. -func (p Params) MarshalJSON() ([]byte, error) { - pDisk := paramsDisk{NotifyUponCompletion: p.NotifyUponCompletion} - return json.Marshal(&pDisk) - -} - -// UnmarshalJSON adheres to the json.Unmarshaler interface. -func (p *Params) UnmarshalJSON(data []byte) error { - pDisk := paramsDisk{} - err := json.Unmarshal(data, &pDisk) - if err != nil { - return err - } - - *p = Params{NotifyUponCompletion: pDisk.NotifyUponCompletion} - - return nil -} diff --git a/fileTransfer/connect/params_test.go b/fileTransfer/connect/params_test.go new file mode 100644 index 0000000000000000000000000000000000000000..2764c111c281c4bdd5c27ab680ccd2b88d42a50e --- /dev/null +++ b/fileTransfer/connect/params_test.go @@ -0,0 +1,98 @@ +//////////////////////////////////////////////////////////////////////////////// +// Copyright © 2022 xx foundation // +// // +// Use of this source code is governed by a license that can be found in the // +// LICENSE file. // +//////////////////////////////////////////////////////////////////////////////// + +package connect + +import ( + "encoding/json" + "reflect" + "testing" +) + +// Tests that DefaultParams returns a Params object with the expected defaults. +func TestDefaultParams(t *testing.T) { + expected := Params{ + NotifyUponCompletion: defaultNotifyUponCompletion, + } + + received := DefaultParams() + if !reflect.DeepEqual(expected, received) { + t.Errorf("Received Params does not match expected."+ + "\nexpected: %+v\nreceived: %+v", expected, received) + } +} + +// Tests that GetParameters uses the passed in parameters. +func TestGetParameters(t *testing.T) { + expected := Params{ + NotifyUponCompletion: false, + } + expectedData, err := json.Marshal(expected) + if err != nil { + t.Errorf("Failed to JSON marshal expected params: %+v", err) + } + + p, err := GetParameters(string(expectedData)) + if err != nil { + t.Errorf("Failed get parameters: %+v", err) + } + + if !reflect.DeepEqual(expected, p) { + t.Errorf("Received Params does not match expected."+ + "\nexpected: %#v\nreceived: %#v", expected, p) + } +} + +// Tests that GetParameters returns the default parameters if no params string +// is provided +func TestGetParameters_Default(t *testing.T) { + expected := DefaultParams() + + p, err := GetParameters("") + if err != nil { + t.Errorf("Failed get parameters: %+v", err) + } + + if !reflect.DeepEqual(expected, p) { + t.Errorf("Received Params does not match expected."+ + "\nexpected: %#v\nreceived: %#v", expected, p) + } +} + +// Error path: Tests that GetParameters returns an error when the params string +// does not contain a valid JSON representation of Params. +func TestGetParameters_InvalidParamsStringError(t *testing.T) { + _, err := GetParameters("invalid JSON") + if err == nil { + t.Error("Failed get get error for invalid JSON") + } +} + +// Tests that a Params object marshalled via json.Marshal and unmarshalled via +// json.Unmarshal matches the original. +func TestParams_JsonMarshalUnmarshal(t *testing.T) { + // Construct a set of params + expected := DefaultParams() + + // Marshal the params + data, err := json.Marshal(&expected) + if err != nil { + t.Fatalf("Marshal error: %v", err) + } + + // Unmarshal the params object + received := Params{} + err = json.Unmarshal(data, &received) + if err != nil { + t.Fatalf("Unmarshal error: %v", err) + } + + if !reflect.DeepEqual(expected, received) { + t.Errorf("Marshalled and unmarshalled Params does not match original."+ + "\nexpected: %#v\nreceived: %#v", expected, received) + } +} diff --git a/fileTransfer/connect/wrapper.go b/fileTransfer/connect/wrapper.go index 135985d0bdcb4613e4f5db7a183e32b2637c4519..c0a89973144d6b338bbadbefccbcffe813b0b112 100644 --- a/fileTransfer/connect/wrapper.go +++ b/fileTransfer/connect/wrapper.go @@ -35,8 +35,8 @@ type Wrapper struct { conn connection } -// connection interface matches a subset of the connect.Connection methods used -// by the Wrapper for easier testing. +// connection interface matches a subset of the [connect.Connection] methods +// used by the Wrapper for easier testing. type connection interface { GetPartner() partner.Manager SendE2E(mt catalog.MessageType, payload []byte, params e2e.Params) ( @@ -118,7 +118,8 @@ func (w *Wrapper) RegisterSentProgressCallback(tid *ftCrypto.TransferID, // addEndMessageToCallback adds the sending of a connection E2E message when // the transfer completed to the callback. If NotifyUponCompletion is not set, // then the message is not sent. -func (w *Wrapper) addEndMessageToCallback(progressCB ft.SentProgressCallback) ft.SentProgressCallback { +func (w *Wrapper) addEndMessageToCallback( + progressCB ft.SentProgressCallback) ft.SentProgressCallback { if !w.p.NotifyUponCompletion { return progressCB } diff --git a/fileTransfer/connect/wrapper_test.go b/fileTransfer/connect/wrapper_test.go index a8f38730cb7885d81713e6c9713bfd348f144075..2a470a19a2945f50f744f9cea4b4f6b0c6c5a435 100644 --- a/fileTransfer/connect/wrapper_test.go +++ b/fileTransfer/connect/wrapper_test.go @@ -25,7 +25,7 @@ import ( "time" ) -// Tests that Connection adheres to the connect.Connection interface. +// Tests that Connection adheres to the [connect.Connection] interface. var _ connection = (connect.Connection)(nil) // Smoke test of the entire file transfer system. @@ -60,7 +60,8 @@ func Test_FileTransfer_Smoke(t *testing.T) { storage1 := newMockStorage() endE2eChan1 := make(chan receive.Message, 3) conn1 := newMockConnection(myID1, myID2, e2eHandler, t) - _, _ = conn1.RegisterListener(catalog.EndFileTransfer, newMockListener(endE2eChan1)) + _, _ = conn1.RegisterListener( + catalog.EndFileTransfer, newMockListener(endE2eChan1)) cMix1 := newMockCmix(myID1, cMixHandler, storage1) user1 := newMockUser(myID1, cMix1, storage1, rngGen) ftManager1, err := ft.NewManager(ftParams, user1) @@ -86,7 +87,8 @@ func Test_FileTransfer_Smoke(t *testing.T) { storage2 := newMockStorage() endE2eChan2 := make(chan receive.Message, 3) conn2 := newMockConnection(myID2, myID1, e2eHandler, t) - _, _ = conn2.RegisterListener(catalog.EndFileTransfer, newMockListener(endE2eChan2)) + _, _ = conn2.RegisterListener( + catalog.EndFileTransfer, newMockListener(endE2eChan2)) cMix2 := newMockCmix(myID1, cMixHandler, storage2) user2 := newMockUser(myID2, cMix2, storage2, rngGen) ftManager2, err := ft.NewManager(ftParams, user2) diff --git a/fileTransfer/e2e/listener.go b/fileTransfer/e2e/listener.go index f2adcf2e2ecc5091a7b4eb1882ad94b90f43435a..ff43c933f16214d1816ab1a7887211b134efc307 100644 --- a/fileTransfer/e2e/listener.go +++ b/fileTransfer/e2e/listener.go @@ -21,8 +21,8 @@ const ( // Name of listener (used for debugging) const listenerName = "NewFileTransferListener-E2E" -// listener waits for a message indicating a new file transfer is starting. -// Adheres to the receive.Listener interface. +// listener waits for a message indicating a new file transfer is starting. This +// structure adheres to the [receive.Listener] interface. type listener struct { m *Wrapper } diff --git a/fileTransfer/e2e/params.go b/fileTransfer/e2e/params.go index 07377414d0d8e084dc39a1a05fead9b40cf987d6..4f8b9995b8f1395fd8f845d2c6b37afa205a314a 100644 --- a/fileTransfer/e2e/params.go +++ b/fileTransfer/e2e/params.go @@ -17,11 +17,7 @@ const ( type Params struct { // NotifyUponCompletion indicates if a final notification message is sent // to the recipient on completion of file transfer. If true, the ping is - NotifyUponCompletion bool -} - -// paramsDisk will be the marshal-able and umarshal-able object. -type paramsDisk struct { + // sent. NotifyUponCompletion bool } @@ -32,8 +28,10 @@ func DefaultParams() Params { } } -// GetParameters returns the default Params, or override with given -// parameters, if set. +// GetParameters returns the default network parameters, or override with given +// parameters, if set. Returns an error if provided invalid JSON. If the JSON is +// valid but does not match the Params structure, the default parameters will be +// returned. func GetParameters(params string) (Params, error) { p := DefaultParams() if len(params) > 0 { @@ -44,23 +42,3 @@ func GetParameters(params string) (Params, error) { } return p, nil } - -// MarshalJSON adheres to the json.Marshaler interface. -func (p Params) MarshalJSON() ([]byte, error) { - pDisk := paramsDisk{NotifyUponCompletion: p.NotifyUponCompletion} - return json.Marshal(&pDisk) - -} - -// UnmarshalJSON adheres to the json.Unmarshaler interface. -func (p *Params) UnmarshalJSON(data []byte) error { - pDisk := paramsDisk{} - err := json.Unmarshal(data, &pDisk) - if err != nil { - return err - } - - *p = Params{NotifyUponCompletion: pDisk.NotifyUponCompletion} - - return nil -} diff --git a/fileTransfer/e2e/params_test.go b/fileTransfer/e2e/params_test.go new file mode 100644 index 0000000000000000000000000000000000000000..9cc7eb1e77d02c0036f7937b73a7e6a3db7280e6 --- /dev/null +++ b/fileTransfer/e2e/params_test.go @@ -0,0 +1,98 @@ +//////////////////////////////////////////////////////////////////////////////// +// Copyright © 2022 xx foundation // +// // +// Use of this source code is governed by a license that can be found in the // +// LICENSE file. // +//////////////////////////////////////////////////////////////////////////////// + +package e2e + +import ( + "encoding/json" + "reflect" + "testing" +) + +// Tests that DefaultParams returns a Params object with the expected defaults. +func TestDefaultParams(t *testing.T) { + expected := Params{ + NotifyUponCompletion: defaultNotifyUponCompletion, + } + + received := DefaultParams() + if !reflect.DeepEqual(expected, received) { + t.Errorf("Received Params does not match expected."+ + "\nexpected: %+v\nreceived: %+v", expected, received) + } +} + +// Tests that GetParameters uses the passed in parameters. +func TestGetParameters(t *testing.T) { + expected := Params{ + NotifyUponCompletion: false, + } + expectedData, err := json.Marshal(expected) + if err != nil { + t.Errorf("Failed to JSON marshal expected params: %+v", err) + } + + p, err := GetParameters(string(expectedData)) + if err != nil { + t.Errorf("Failed get parameters: %+v", err) + } + + if !reflect.DeepEqual(expected, p) { + t.Errorf("Received Params does not match expected."+ + "\nexpected: %#v\nreceived: %#v", expected, p) + } +} + +// Tests that GetParameters returns the default parameters if no params string +// is provided +func TestGetParameters_Default(t *testing.T) { + expected := DefaultParams() + + p, err := GetParameters("") + if err != nil { + t.Errorf("Failed get parameters: %+v", err) + } + + if !reflect.DeepEqual(expected, p) { + t.Errorf("Received Params does not match expected."+ + "\nexpected: %#v\nreceived: %#v", expected, p) + } +} + +// Error path: Tests that GetParameters returns an error when the params string +// does not contain a valid JSON representation of Params. +func TestGetParameters_InvalidParamsStringError(t *testing.T) { + _, err := GetParameters("invalid JSON") + if err == nil { + t.Error("Failed get get error for invalid JSON") + } +} + +// Tests that a Params object marshalled via json.Marshal and unmarshalled via +// json.Unmarshal matches the original. +func TestParams_JsonMarshalUnmarshal(t *testing.T) { + // Construct a set of params + expected := DefaultParams() + + // Marshal the params + data, err := json.Marshal(&expected) + if err != nil { + t.Fatalf("Marshal error: %v", err) + } + + // Unmarshal the params object + received := Params{} + err = json.Unmarshal(data, &received) + if err != nil { + t.Fatalf("Unmarshal error: %v", err) + } + + if !reflect.DeepEqual(expected, received) { + t.Errorf("Marshalled and unmarshalled Params does not match original."+ + "\nexpected: %#v\nreceived: %#v", expected, received) + } +} diff --git a/fileTransfer/e2e/send.go b/fileTransfer/e2e/send.go index 2aa2b5a9bf84b528071f0540b8d076c6db26afd0..653e59b647cfbd0bf39bf3b6154db2d2d3e696ed 100644 --- a/fileTransfer/e2e/send.go +++ b/fileTransfer/e2e/send.go @@ -57,7 +57,8 @@ func sendNewFileTransferMessage( // sendEndFileTransferMessage sends an E2E message to the recipient informing // them that all file parts have arrived once the network is healthy. -func sendEndFileTransferMessage(recipient *id.ID, cmix ft.Cmix, e2eHandler e2eHandler) { +func sendEndFileTransferMessage( + recipient *id.ID, cmix ft.Cmix, e2eHandler e2eHandler) { callbackID := make(chan uint64, 1) callbackID <- cmix.AddHealthCallback( func(healthy bool) { diff --git a/fileTransfer/e2e/wrapper.go b/fileTransfer/e2e/wrapper.go index ac575583b6ecb817dfdccad3aed61abc6db3fe5b..fa59905cef8c5efffae9e514ab18562f2aadb6f0 100644 --- a/fileTransfer/e2e/wrapper.go +++ b/fileTransfer/e2e/wrapper.go @@ -35,8 +35,8 @@ type Wrapper struct { e2e e2eHandler } -// e2eHandler interface matches a subset of the e2e.Handler methods used by the Wrapper -// for easier testing. +// e2eHandler interface matches a subset of the [e2e.Handler] methods used by +// the Wrapper for easier testing. type e2eHandler interface { SendE2E(mt catalog.MessageType, recipient *id.ID, payload []byte, params e2e.Params) (cryptoE2e.SendReport, error) diff --git a/fileTransfer/groupChat/wrapper.go b/fileTransfer/groupChat/wrapper.go index 27de9dd383702f90b8e2f7cf81d8a3b7ba8a707b..8566d645026bb5c3335cae6680cfdb4666576005 100644 --- a/fileTransfer/groupChat/wrapper.go +++ b/fileTransfer/groupChat/wrapper.go @@ -49,8 +49,8 @@ type gcManager interface { } // NewWrapper generates a new file transfer Wrapper for group chat. -func NewWrapper(receiveCB ft.ReceiveCallback, ft ft.FileTransfer, gc gcManager) ( - *Wrapper, error) { +func NewWrapper(receiveCB ft.ReceiveCallback, ft ft.FileTransfer, + gc gcManager) (*Wrapper, error) { w := &Wrapper{ receiveCB: receiveCB, ft: ft, diff --git a/fileTransfer/manager_test.go b/fileTransfer/manager_test.go index c2bee63b482f66b057625c06d2c53a601f1f6620..80e6bec4f19f00badcf5a03011dff76233288414 100644 --- a/fileTransfer/manager_test.go +++ b/fileTransfer/manager_test.go @@ -204,9 +204,11 @@ func Test_FileTransfer_Smoke(t *testing.T) { fileName, sendTime, fileSizeKb, throughput) expectedThroughput := float64(params.MaxThroughput) * .001 - delta := (math.Abs(expectedThroughput-throughput) / ((expectedThroughput + throughput) / 2)) * 100 + delta := (math.Abs(expectedThroughput-throughput) / + ((expectedThroughput + throughput) / 2)) * 100 t.Logf("Expected bandwidth: %.2f kb/s", expectedThroughput) - t.Logf("Bandwidth difference: %.2f kb/s (%.2f%%)", expectedThroughput-throughput, delta) + t.Logf("Bandwidth difference: %.2f kb/s (%.2f%%)", + expectedThroughput-throughput, delta) } }() diff --git a/fileTransfer/params.go b/fileTransfer/params.go index e0ecffed8ed3d040cc4c2f4a54d04f4b34f26b83..b4988da4de909721cc81602f6f2c460aa513b5b7 100644 --- a/fileTransfer/params.go +++ b/fileTransfer/params.go @@ -33,13 +33,6 @@ type Params struct { Cmix cmix.CMIXParams } -// paramsDisk will be the marshal-able and umarshal-able object. -type paramsDisk struct { - MaxThroughput int - SendTimeout time.Duration - Cmix cmix.CMIXParams -} - // DefaultParams returns a Params object filled with the default values. func DefaultParams() Params { return Params{ @@ -50,7 +43,9 @@ func DefaultParams() Params { } // GetParameters returns the default network parameters, or override with given -// parameters, if set. +// parameters, if set. Returns an error if provided invalid JSON. If the JSON is +// valid but does not match the Params structure, the default parameters will be +// returned. func GetParameters(params string) (Params, error) { p := DefaultParams() if len(params) > 0 { @@ -61,32 +56,3 @@ func GetParameters(params string) (Params, error) { } return p, nil } - -// MarshalJSON adheres to the json.Marshaler interface. -func (p Params) MarshalJSON() ([]byte, error) { - pDisk := paramsDisk{ - MaxThroughput: p.MaxThroughput, - SendTimeout: p.SendTimeout, - Cmix: p.Cmix, - } - - return json.Marshal(&pDisk) - -} - -// UnmarshalJSON adheres to the json.Unmarshaler interface. -func (p *Params) UnmarshalJSON(data []byte) error { - pDisk := paramsDisk{} - err := json.Unmarshal(data, &pDisk) - if err != nil { - return err - } - - *p = Params{ - MaxThroughput: pDisk.MaxThroughput, - SendTimeout: pDisk.SendTimeout, - Cmix: pDisk.Cmix, - } - - return nil -} diff --git a/fileTransfer/params_test.go b/fileTransfer/params_test.go index e60c9a6381c75881c049d6b538f4b3587aac4ece..23e92d46a0bc98aa86034506cb9c25a3fa45b138 100644 --- a/fileTransfer/params_test.go +++ b/fileTransfer/params_test.go @@ -8,58 +8,111 @@ package fileTransfer import ( - "bytes" "encoding/json" "gitlab.com/elixxir/client/cmix" "reflect" "testing" ) -// Tests that no data is lost when marshaling and unmarshalling the Params -// object. -func TestParams_MarshalUnmarshal(t *testing.T) { - // Construct a set of params - p := DefaultParams() +// Tests that DefaultParams returns a Params object with the expected defaults. +func TestDefaultParams(t *testing.T) { + expected := Params{ + MaxThroughput: defaultMaxThroughput, + SendTimeout: defaultSendTimeout, + Cmix: cmix.GetDefaultCMIXParams(), + } + received := DefaultParams() + received.Cmix.Stop = expected.Cmix.Stop - // Marshal the params - data, err := json.Marshal(&p) + if !reflect.DeepEqual(expected, received) { + t.Errorf("Received Params does not match expected."+ + "\nexpected: %+v\nreceived: %+v", expected, received) + } +} + +// Tests that GetParameters uses the passed in parameters. +func TestGetParameters(t *testing.T) { + expected := Params{ + MaxThroughput: 42, + SendTimeout: 11, + Cmix: cmix.CMIXParams{ + RoundTries: 5, + Timeout: 6, + RetryDelay: 7, + ExcludedRounds: nil, + SendTimeout: 8, + DebugTag: "9", + Stop: nil, + BlacklistedNodes: cmix.NodeMap{}, + Critical: true, + }, + } + expectedData, err := json.Marshal(expected) if err != nil { - t.Fatalf("Marshal error: %v", err) + t.Errorf("Failed to JSON marshal expected params: %+v", err) } - // Unmarshal the params object - received := Params{} - err = json.Unmarshal(data, &received) + p, err := GetParameters(string(expectedData)) if err != nil { - t.Fatalf("Unmarshal error: %v", err) + t.Errorf("Failed get parameters: %+v", err) } + p.Cmix.Stop = expected.Cmix.Stop + + if !reflect.DeepEqual(expected, p) { + t.Errorf("Received Params does not match expected."+ + "\nexpected: %#v\nreceived: %#v", expected, p) + } +} - // Re-marshal this params object - data2, err := json.Marshal(received) +// Tests that GetParameters returns the default parameters if no params string +// is provided +func TestGetParameters_Default(t *testing.T) { + expected := DefaultParams() + + p, err := GetParameters("") if err != nil { - t.Fatalf("Marshal error: %v", err) + t.Errorf("Failed get parameters: %+v", err) } + p.Cmix.Stop = expected.Cmix.Stop - // Check that they match (it is done this way to avoid false failures with - // the reflect.DeepEqual function and pointers) - if !bytes.Equal(data, data2) { - t.Fatalf("Data was lost in marshal/unmarshal.") + if !reflect.DeepEqual(expected, p) { + t.Errorf("Received Params does not match expected."+ + "\nexpected: %#v\nreceived: %#v", expected, p) } +} +// Error path: Tests that GetParameters returns an error when the params string +// does not contain a valid JSON representation of Params. +func TestGetParameters_InvalidParamsStringError(t *testing.T) { + _, err := GetParameters("invalid JSON") + if err == nil { + t.Error("Failed get get error for invalid JSON") + } } -// Tests that DefaultParams returns a Params object with the expected defaults. -func TestDefaultParams(t *testing.T) { - expected := Params{ - MaxThroughput: defaultMaxThroughput, - SendTimeout: defaultSendTimeout, - Cmix: cmix.GetDefaultCMIXParams(), +// Tests that a Params object marshalled via json.Marshal and unmarshalled via +// json.Unmarshal matches the original. +func TestParams_JsonMarshalUnmarshal(t *testing.T) { + // Construct a set of params + expected := DefaultParams() + expected.Cmix.BlacklistedNodes = cmix.NodeMap{} + + // Marshal the params + data, err := json.Marshal(&expected) + if err != nil { + t.Fatalf("Marshal error: %v", err) + } + + // Unmarshal the params object + received := Params{} + err = json.Unmarshal(data, &received) + if err != nil { + t.Fatalf("Unmarshal error: %v", err) } - received := DefaultParams() - received.Cmix.Stop = expected.Cmix.Stop + received.Cmix.Stop = expected.Cmix.Stop if !reflect.DeepEqual(expected, received) { - t.Errorf("Received Params does not match expected."+ - "\nexpected: %+v\nreceived: %+v", expected, received) + t.Errorf("Marshalled and unmarshalled Params does not match original."+ + "\nexpected: %#v\nreceived: %#v", expected, received) } }