From 0b595e03932f4d8fcced6de84f488fa901f7447a Mon Sep 17 00:00:00 2001 From: y-du <42994541+y-du@users.noreply.github.com> Date: Fri, 19 Jul 2024 10:21:08 +0200 Subject: [PATCH] refactor and improve logging --- handler/itf.go | 2 +- handler/message_hdl/handler.go | 35 ++++++++++------- handler/message_hdl/handler_test.go | 55 ++++++++++++++++----------- handler/msg_relay_hdl/handler.go | 8 +--- handler/msg_relay_hdl/handler_test.go | 30 +-------------- 5 files changed, 57 insertions(+), 73 deletions(-) diff --git a/handler/itf.go b/handler/itf.go index 74e2520..d1f8a1e 100644 --- a/handler/itf.go +++ b/handler/itf.go @@ -41,4 +41,4 @@ type MessageRelayHandler interface { Put(m Message) error } -type MessageHandler func(m Message) error +type MessageHandler func(m Message) diff --git a/handler/message_hdl/handler.go b/handler/message_hdl/handler.go index 7346556..cd7e311 100644 --- a/handler/message_hdl/handler.go +++ b/handler/message_hdl/handler.go @@ -3,13 +3,14 @@ package message_hdl import ( "context" "encoding/json" - "errors" - "fmt" "github.com/SENERGY-Platform/mgw-device-manager/handler" lib_model "github.com/SENERGY-Platform/mgw-device-manager/lib/model" + "github.com/SENERGY-Platform/mgw-device-manager/util" "github.com/SENERGY-Platform/mgw-device-manager/util/topic" ) +const logPrefix = "[message-hdl]" + type Handler struct { devicesHdl handler.DevicesHandler } @@ -18,18 +19,21 @@ func New(devicesHdl handler.DevicesHandler) *Handler { return &Handler{devicesHdl: devicesHdl} } -func (h *Handler) HandleMessage(m handler.Message) error { +func (h *Handler) HandleMessage(m handler.Message) { var ref string switch { case parseTopic(topic.DevicesSub, m.Topic(), &ref): var dm lib_model.DeviceMessage if err := json.Unmarshal(m.Payload(), &dm); err != nil { - return err + util.Logger.Errorf("%s unmarshal message: %s", logPrefix, err) + return } switch dm.Method { case lib_model.Set: + util.Logger.Infof("%s set device (%s)", logPrefix, dm.DeviceID) if dm.Data == nil { - return errors.New("missing device data") + util.Logger.Errorf("%s set device (%s): missing data", logPrefix, dm.DeviceID) + return } err := h.devicesHdl.Put(context.Background(), lib_model.DeviceData{ ID: dm.DeviceID, @@ -40,20 +44,23 @@ func (h *Handler) HandleMessage(m handler.Message) error { Attributes: dm.Data.Attributes, }) if err != nil { - var iie *lib_model.InvalidInputError - if errors.As(err, &iie) { - return err - } + util.Logger.Errorf("%s set device (%s): %s", logPrefix, dm.DeviceID, err) } case lib_model.Delete: - _ = h.devicesHdl.Delete(context.Background(), dm.DeviceID) + util.Logger.Infof("%s delete device (%s)", logPrefix, dm.DeviceID) + if err := h.devicesHdl.Delete(context.Background(), dm.DeviceID); err != nil { + util.Logger.Errorf("%s delete device (%s): %s", logPrefix, dm.DeviceID, err) + } default: - return fmt.Errorf("unknown method '%s'", dm.Method) + util.Logger.Errorf("%s unknown method '%s'", logPrefix, dm.Method) } case parseTopic(topic.LastWillSub, m.Topic(), &ref): - _ = h.devicesHdl.SetStates(context.Background(), ref, lib_model.Offline) + util.Logger.Infof("%s set device states (%s)", logPrefix, ref) + if err := h.devicesHdl.SetStates(context.Background(), ref, lib_model.Offline); err != nil { + util.Logger.Errorf("%s set device states (%s): %s", logPrefix, ref, err) + } default: - return fmt.Errorf("parsing topic '%s' failed", m.Topic()) + util.Logger.Errorf("%s unknown topic '%s'", logPrefix, m.Topic()) } - return nil + return } diff --git a/handler/message_hdl/handler_test.go b/handler/message_hdl/handler_test.go index e3958c5..5cc7e36 100644 --- a/handler/message_hdl/handler_test.go +++ b/handler/message_hdl/handler_test.go @@ -4,12 +4,15 @@ import ( "context" "encoding/json" "errors" + sb_util "github.com/SENERGY-Platform/go-service-base/util" lib_model "github.com/SENERGY-Platform/mgw-device-manager/lib/model" + "github.com/SENERGY-Platform/mgw-device-manager/util" "reflect" "testing" ) func TestHandler_HandleMessage(t *testing.T) { + util.InitLogger(sb_util.LoggerConfig{Terminal: true, Level: 4}) t.Run("set device", func(t *testing.T) { mockDHdl := &mockDeviceHdl{ Devices: make(map[string]lib_model.DeviceData), @@ -46,13 +49,10 @@ func TestHandler_HandleMessage(t *testing.T) { if err != nil { t.Fatal(err) } - err = h.HandleMessage(&mockMessage{ + h.HandleMessage(&mockMessage{ topic: "device-manager/device/test", payload: p, }) - if err != nil { - t.Error(err) - } b, ok := mockDHdl.Devices["123"] if !ok { t.Error("not in map") @@ -70,24 +70,21 @@ func TestHandler_HandleMessage(t *testing.T) { Method: lib_model.Set, DeviceID: "123", }) - err = h.HandleMessage(&mockMessage{ + if err != nil { + t.Fatal(err) + } + h.HandleMessage(&mockMessage{ topic: "device-manager/device/test", payload: p2, }) - if err == nil { - t.Error("expected error") - } }) t.Run("error", func(t *testing.T) { - mockDHdl := &mockDeviceHdl{PutErr: lib_model.NewInvalidInputError(errors.New("test"))} + mockDHdl := &mockDeviceHdl{PutErr: errors.New("test")} h := Handler{devicesHdl: mockDHdl} - err = h.HandleMessage(&mockMessage{ + h.HandleMessage(&mockMessage{ topic: "device-manager/device/test", payload: p, }) - if err == nil { - t.Error("expected error") - } }) }) t.Run("delete device", func(t *testing.T) { @@ -100,20 +97,28 @@ func TestHandler_HandleMessage(t *testing.T) { if err != nil { t.Fatal(err) } - _ = h.HandleMessage(&mockMessage{ + h.HandleMessage(&mockMessage{ topic: "device-manager/device/test", payload: p, }) if mockDHdl.DeleteC != 1 { t.Error("missing call") } + t.Run("error", func(t *testing.T) { + mockDHdl := &mockDeviceHdl{DeleteErr: errors.New("test")} + h := Handler{devicesHdl: mockDHdl} + h.HandleMessage(&mockMessage{ + topic: "device-manager/device/test", + payload: p, + }) + }) }) t.Run("set states", func(t *testing.T) { mockDHdl := &mockDeviceHdl{ States: make(map[string]lib_model.DeviceState), } h := Handler{devicesHdl: mockDHdl} - _ = h.HandleMessage(&mockMessage{ + h.HandleMessage(&mockMessage{ topic: "device-manager/device/test/lw", }) s, ok := mockDHdl.States["test"] @@ -123,6 +128,13 @@ func TestHandler_HandleMessage(t *testing.T) { if s != lib_model.Offline { t.Error("got", s, "expected", lib_model.Offline) } + t.Run("error", func(t *testing.T) { + mockDHdl := &mockDeviceHdl{SetStatesErr: errors.New("test")} + h := Handler{devicesHdl: mockDHdl} + h.HandleMessage(&mockMessage{ + topic: "device-manager/device/test/lw", + }) + }) }) t.Run("unknown method", func(t *testing.T) { mockDHdl := &mockDeviceHdl{} @@ -133,23 +145,17 @@ func TestHandler_HandleMessage(t *testing.T) { if err != nil { t.Fatal(err) } - err = h.HandleMessage(&mockMessage{ + h.HandleMessage(&mockMessage{ topic: "device-manager/device/test", payload: p, }) - if err == nil { - t.Error("expected error") - } }) t.Run("parse topic error", func(t *testing.T) { mockDHdl := &mockDeviceHdl{} h := Handler{devicesHdl: mockDHdl} - err := h.HandleMessage(&mockMessage{ + h.HandleMessage(&mockMessage{ topic: "test", }) - if err == nil { - t.Error("expected error") - } }) } @@ -196,6 +202,9 @@ func (m *mockDeviceHdl) SetStates(ctx context.Context, ref string, state lib_mod func (m *mockDeviceHdl) Delete(ctx context.Context, id string) error { m.DeleteC++ + if m.DeleteErr != nil { + return m.DeleteErr + } return nil } diff --git a/handler/msg_relay_hdl/handler.go b/handler/msg_relay_hdl/handler.go index e433b5e..d5f8483 100644 --- a/handler/msg_relay_hdl/handler.go +++ b/handler/msg_relay_hdl/handler.go @@ -3,11 +3,8 @@ package msg_relay_hdl import ( "errors" "github.com/SENERGY-Platform/mgw-device-manager/handler" - "github.com/SENERGY-Platform/mgw-device-manager/util" ) -const logPrefix = "[relay-hdl]" - type Handler struct { messages chan handler.Message handleFunc handler.MessageHandler @@ -42,10 +39,7 @@ func (h *Handler) Stop() { func (h *Handler) run() { for message := range h.messages { - err := h.handleFunc(message) - if err != nil { - util.Logger.Errorf("%s handle message: %s", logPrefix, err) - } + h.handleFunc(message) } h.dChan <- struct{}{} } diff --git a/handler/msg_relay_hdl/handler_test.go b/handler/msg_relay_hdl/handler_test.go index 03b5c6a..28fcf07 100644 --- a/handler/msg_relay_hdl/handler_test.go +++ b/handler/msg_relay_hdl/handler_test.go @@ -1,10 +1,7 @@ package msg_relay_hdl import ( - "errors" - sb_util "github.com/SENERGY-Platform/go-service-base/util" "github.com/SENERGY-Platform/mgw-device-manager/handler" - "github.com/SENERGY-Platform/mgw-device-manager/util" "reflect" "testing" "time" @@ -25,17 +22,15 @@ func (m *mockMessage) Payload() []byte { } func TestHandler(t *testing.T) { - util.InitLogger(sb_util.LoggerConfig{Terminal: true, Level: 4}) msg := &mockMessage{ topic: "test", payload: []byte("test"), timestamp: time.Now(), } - testMsgHdl := func(m handler.Message) error { + testMsgHdl := func(m handler.Message) { if !reflect.DeepEqual(m, msg) { t.Error("expected", msg, "got", m) } - return nil } h := New(1, testMsgHdl) err := h.Put(msg) @@ -51,29 +46,8 @@ func TestHandler(t *testing.T) { t.Error("message not consumed") } h.Stop() - t.Run("message handler error", func(t *testing.T) { - testMsgHdl = func(m handler.Message) error { - return errors.New("test error") - } - h = New(1, testMsgHdl) - err = h.Put(msg) - if err != nil { - t.Error(err) - } - if len(h.messages) != 1 { - t.Error("message not in channel") - } - h.Start() - time.Sleep(1 * time.Second) - if len(h.messages) > 0 { - t.Error("message not consumed") - } - h.Stop() - }) t.Run("buffer full", func(t *testing.T) { - testMsgHdl = func(m handler.Message) error { - return nil - } + testMsgHdl = func(m handler.Message) {} h = New(1, testMsgHdl) err = h.Put(msg) if err != nil {