Skip to content

Commit

Permalink
Fix the Content Header-related issue with CBOR.
Browse files Browse the repository at this point in the history
Signed-off-by: Akram Ahmad <sftwr2020@gmail.com>
  • Loading branch information
akramtexas committed Apr 9, 2020
1 parent 6d36f33 commit c997980
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 32 deletions.
3 changes: 3 additions & 0 deletions example/cmd/device-simple/Attribution.txt
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ https://github.com/pmezard/go-difflib/blob/master/LICENSE
stretchr/testify (MIT) https://github.com/stretchr/testify
https://github.com/stretchr/testify/blob/master/LICENSE

ugorji/go (MIT) https://github.com/ugorji/go
https://github.com/ugorji/go/blob/master/LICENSE

kr/logfmt (MIT) https://github.com/kr/logfmt
https://github.com/kr/logfmt/blob/master/Readme

Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ require (
github.com/gorilla/mux v1.7.1
github.com/pkg/errors v0.8.1
github.com/stretchr/testify v1.5.1
github.com/ugorji/go v1.1.4
gopkg.in/yaml.v2 v2.2.8
)

Expand Down
5 changes: 3 additions & 2 deletions internal/autoevent/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ import (
"sync"
"time"

"github.com/OneOfOne/xxhash"
"github.com/edgexfoundry/device-sdk-go/internal/common"
"github.com/edgexfoundry/device-sdk-go/internal/handler"
dsModels "github.com/edgexfoundry/device-sdk-go/pkg/models"
contract "github.com/edgexfoundry/go-mod-core-contracts/models"

"github.com/OneOfOne/xxhash"
)

type Executor interface {
Expand Down Expand Up @@ -84,7 +85,7 @@ func readResource(e *executor) (*dsModels.Event, common.AppError) {
vars[common.NameVar] = e.deviceName
vars[common.CommandVar] = e.autoEvent.Resource

evt, appErr := handler.CommandHandler(vars, "", common.GetCmdMethod, "")
evt, appErr := handler.CommandHandler(vars, "", nil)
return evt, appErr
}

Expand Down
3 changes: 2 additions & 1 deletion internal/common/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import (
// WritableInfo is a struct which contains configuration settings that can be changed in the Registry .
type WritableInfo struct {
// Level is the logging level of writing log message
LogLevel string
LogLevel string
ChecksumAlgo string
}

// ServiceInfo is a struct which contains service related configuration
Expand Down
5 changes: 3 additions & 2 deletions internal/controller/restfuncs.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/edgexfoundry/device-sdk-go/internal/handler/callback"
"github.com/edgexfoundry/go-mod-core-contracts/clients"
contract "github.com/edgexfoundry/go-mod-core-contracts/models"

"github.com/gorilla/mux"
)

Expand Down Expand Up @@ -105,7 +106,7 @@ func commandFunc(w http.ResponseWriter, req *http.Request) {
return
}

event, appErr := handler.CommandHandler(vars, body, req.Method, req.URL.RawQuery)
event, appErr := handler.CommandHandler(vars, body, req)

if appErr != nil {
http.Error(w, fmt.Sprintf("%s %s", appErr.Message(), req.URL.Path), appErr.Code())
Expand Down Expand Up @@ -149,7 +150,7 @@ func commandAllFunc(w http.ResponseWriter, req *http.Request) {
return
}

events, appErr := handler.CommandAllHandler(vars[common.CommandVar], body, req.Method, req.URL.RawQuery)
events, appErr := handler.CommandAllHandler(vars[common.CommandVar], body, req)
if appErr != nil {
http.Error(w, appErr.Message(), appErr.Code())
} else if len(events) > 0 {
Expand Down
85 changes: 69 additions & 16 deletions internal/handler/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ package handler
import (
"encoding/json"
"fmt"
"net/http"
"strconv"
"strings"
"sync"
Expand All @@ -19,12 +20,23 @@ import (
"github.com/edgexfoundry/device-sdk-go/internal/common"
"github.com/edgexfoundry/device-sdk-go/internal/transformer"
dsModels "github.com/edgexfoundry/device-sdk-go/pkg/models"

"github.com/edgexfoundry/go-mod-core-contracts/clients"
contract "github.com/edgexfoundry/go-mod-core-contracts/models"

"github.com/ugorji/go/codec"
)

// Note, every HTTP request to ServeHTTP is made in a separate goroutine, which
// means care needs to be taken with respect to shared data accessed through *Server.
func CommandHandler(vars map[string]string, body string, method string, queryParams string) (*dsModels.Event, common.AppError) {
func CommandHandler(vars map[string]string, body string, req *http.Request) (*dsModels.Event, common.AppError) {
if req == nil {
return nil, common.NewNotFoundError("error in request parameter that was passed in", nil)
}

method := req.Method
queryParams := req.URL.RawQuery

dKey := vars[common.IdVar]
cmd := vars[common.CommandVar]

Expand Down Expand Up @@ -78,21 +90,25 @@ func CommandHandler(vars map[string]string, body string, method string, queryPar
if strings.ToLower(method) == common.GetCmdMethod {
evt, appErr = execReadDeviceResource(&d, &dr, queryParams)
} else {
appErr = execWriteDeviceResource(&d, &dr, body)
appErr = execWriteDeviceResource(&d, &dr, body, req)
}
} else {
if strings.ToLower(method) == common.GetCmdMethod {
evt, appErr = execReadCmd(&d, cmd, queryParams)
} else {
appErr = execWriteCmd(&d, cmd, body)
appErr = execWriteCmd(&d, cmd, body, req)
}
}

go common.UpdateLastConnected(d.Name)
return evt, appErr
}

func execReadDeviceResource(device *contract.Device, dr *contract.DeviceResource, queryParams string) (*dsModels.Event, common.AppError) {
func execReadDeviceResource(
device *contract.Device,
dr *contract.DeviceResource,
queryParams string) (*dsModels.Event, common.AppError) {

var reqs []dsModels.CommandRequest
var req dsModels.CommandRequest
common.LoggingClient.Debug(fmt.Sprintf("Handler - execReadCmd: deviceResource: %s", dr.Name))
Expand Down Expand Up @@ -249,8 +265,13 @@ func execReadCmd(device *contract.Device, cmd string, queryParams string) (*dsMo
return cvsToEvent(device, results, cmd)
}

func execWriteDeviceResource(device *contract.Device, dr *contract.DeviceResource, params string) common.AppError {
paramMap, err := parseParams(params)
func execWriteDeviceResource(
device *contract.Device,
dr *contract.DeviceResource,
params string,
r *http.Request) common.AppError {

paramMap, err := parseParams(params, r)
if err != nil {
msg := fmt.Sprintf("Handler - execWriteDeviceResource: Put parameters parsing failed: %s", params)
common.LoggingClient.Error(msg)
Expand Down Expand Up @@ -297,7 +318,7 @@ func execWriteDeviceResource(device *contract.Device, dr *contract.DeviceResourc
return nil
}

func execWriteCmd(device *contract.Device, cmd string, params string) common.AppError {
func execWriteCmd(device *contract.Device, cmd string, params string, r *http.Request) common.AppError {
ros, err := cache.Profiles().ResourceOperations(device.Profile.Name, cmd, common.SetCmdMethod)
if err != nil {
msg := fmt.Sprintf("Handler - execWriteCmd: can't find ResrouceOperations in Profile(%s) and Command(%s), %v", device.Profile.Name, cmd, err)
Expand All @@ -312,7 +333,7 @@ func execWriteCmd(device *contract.Device, cmd string, params string) common.App
return common.NewServerError(msg, nil)
}

cvs, err := parseWriteParams(device.Profile.Name, ros, params)
cvs, err := parseWriteParams(device.Profile.Name, ros, params, r)
if err != nil {
msg := fmt.Sprintf("Handler - execWriteCmd: Put parameters parsing failed: %s", params)
common.LoggingClient.Error(msg)
Expand Down Expand Up @@ -359,8 +380,12 @@ func execWriteCmd(device *contract.Device, cmd string, params string) common.App
return nil
}

func parseWriteParams(profileName string, ros []contract.ResourceOperation, params string) ([]*dsModels.CommandValue, error) {
paramMap, err := parseParams(params)
func parseWriteParams(profileName string,
ros []contract.ResourceOperation,
params string,
r *http.Request) ([]*dsModels.CommandValue, error) {

paramMap, err := parseParams(params, r)
if err != nil {
return []*dsModels.CommandValue{}, err
}
Expand Down Expand Up @@ -410,8 +435,22 @@ func parseWriteParams(profileName string, ros []contract.ResourceOperation, para
return result, nil
}

func parseParams(params string) (paramMap map[string]string, err error) {
err = json.Unmarshal([]byte(params), &paramMap)
func parseParams(params string, r *http.Request) (paramMap map[string]string, err error) {

if r == nil {
return nil, fmt.Errorf("error in request parameter that was passed in")
}

// Check the header value
switch r.Header.Get(clients.ContentType) {
case clients.ContentTypeCBOR:
err = codec.NewDecoderBytes([]byte(params), &codec.CborHandle{}).Decode(&paramMap)
case clients.ContentTypeJSON:
err = json.Unmarshal([]byte(params), &paramMap)
default:
common.LoggingClient.Error(fmt.Sprintf("header value was neither JSON nor CBOR, instead was: %s", r.Header.Get(clients.ContentType)))
}

if err != nil {
common.LoggingClient.Error(fmt.Sprintf("parsing Write parameters failed %s, %v", params, err))
return
Expand All @@ -424,7 +463,11 @@ func parseParams(params string) (paramMap map[string]string, err error) {
return
}

func createCommandValueFromRO(profileName string, ro *contract.ResourceOperation, v string) (*dsModels.CommandValue, error) {
func createCommandValueFromRO(
profileName string,
ro *contract.ResourceOperation,
v string) (*dsModels.CommandValue, error) {

dr, ok := cache.Profiles().DeviceResource(profileName, ro.DeviceResource)
if !ok {
msg := fmt.Sprintf("createCommandValueForParam: no deviceResource: %s", ro.DeviceResource)
Expand Down Expand Up @@ -505,8 +548,18 @@ func createCommandValueFromDR(dr *contract.DeviceResource, v string) (*dsModels.
return result, err
}

func CommandAllHandler(cmd string, body string, method string, queryParams string) ([]*dsModels.Event, common.AppError) {
common.LoggingClient.Debug(fmt.Sprintf("Handler - CommandAll: execute the %s command %s from all operational devices", method, cmd))
func CommandAllHandler(cmd string, body string, req *http.Request) ([]*dsModels.Event, common.AppError) {
if req == nil {
return nil, common.NewNotFoundError("error in request parameter that was passed in", nil)
}

method := req.Method
queryParams := req.URL.RawQuery

common.LoggingClient.Debug(fmt.Sprintf(
"Handler - CommandAll: execute the %s command %s from all operational devices",
method,
cmd))
devices := filterOperationalDevices(cache.Devices().All())

devCount := len(devices)
Expand All @@ -525,7 +578,7 @@ func CommandAllHandler(cmd string, body string, method string, queryParams strin
if strings.ToLower(method) == common.GetCmdMethod {
event, appErr = execReadCmd(device, cmd, queryParams)
} else {
appErr = execWriteCmd(device, cmd, body)
appErr = execWriteCmd(device, cmd, body, req)
}
cmdResults <- struct {
event *dsModels.Event
Expand Down
41 changes: 30 additions & 11 deletions internal/handler/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,23 @@ package handler

import (
"context"
"net/http"
"net/http/httptest"
"strconv"
"strings"
"testing"

"github.com/edgexfoundry/go-mod-core-contracts/clients/logger"
contract "github.com/edgexfoundry/go-mod-core-contracts/models"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"

"github.com/edgexfoundry/device-sdk-go/internal/cache"
"github.com/edgexfoundry/device-sdk-go/internal/common"
"github.com/edgexfoundry/device-sdk-go/internal/mock"
dsModels "github.com/edgexfoundry/device-sdk-go/pkg/models"

"github.com/edgexfoundry/go-mod-core-contracts/clients"
"github.com/edgexfoundry/go-mod-core-contracts/clients/logger"
contract "github.com/edgexfoundry/go-mod-core-contracts/models"

"github.com/google/uuid"
"github.com/stretchr/testify/assert"
)

const (
Expand Down Expand Up @@ -83,8 +87,9 @@ func TestParseWriteParamsWrongParamName(t *testing.T) {
profileName := "notFound"
ro := []contract.ResourceOperation{{Index: ""}}
params := "{ \"key\": \"value\" }"
r := newRequestWithContentType(clients.ContentTypeJSON)

_, err := parseWriteParams(profileName, ro, params)
_, err := parseWriteParams(profileName, ro, params, r)

if err == nil {
t.Error("expected error")
Expand All @@ -96,7 +101,8 @@ func TestParseWriteParamsNoParams(t *testing.T) {
ro := []contract.ResourceOperation{{Index: ""}}
params := "{ }"

_, err := parseWriteParams(profileName, ro, params)
r := newRequestWithContentType(clients.ContentTypeJSON)
_, err := parseWriteParams(profileName, ro, params, r)

if err == nil {
t.Error("expected error")
Expand Down Expand Up @@ -237,6 +243,7 @@ func TestParseWriteParams(t *testing.T) {
rosTestDefaultValue, _ := cache.Profiles().ResourceOperations(profileName, "RandomValue_Int32", common.SetCmdMethod)
rosTestMappingPass, _ := cache.Profiles().ResourceOperations(profileName, "ResourceTestMapping_Pass", common.SetCmdMethod)
rosTestMappingFail, _ := cache.Profiles().ResourceOperations(profileName, "ResourceTestMapping_Fail", common.SetCmdMethod)
r := newRequestWithContentType(clients.ContentTypeJSON)

tests := []struct {
testName string
Expand All @@ -258,7 +265,7 @@ func TestParseWriteParams(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.testName, func(t *testing.T) {
_, err := parseWriteParams(tt.profile, tt.resourceOps, tt.params)
_, err := parseWriteParams(tt.profile, tt.resourceOps, tt.params, r)
if !tt.expectErr && err != nil {
t.Errorf("unexpected parse error params:%s %s", tt.params, err.Error())
return
Expand Down Expand Up @@ -335,6 +342,9 @@ func TestExecWriteCmd(t *testing.T) {
paramsTransformFail = `{"ResourceTestTransform_Fail":"123"}`
paramsNoDeviceResourceForResult = `{"error":""}`
)

r := newRequestWithContentType(clients.ContentTypeJSON)

tests := []struct {
testName string
device *contract.Device
Expand All @@ -358,7 +368,7 @@ func TestExecWriteCmd(t *testing.T) {
common.CurrentConfig.Device.MaxCmdOps = 128
}()
}
appErr := execWriteCmd(tt.device, tt.cmd, tt.params)
appErr := execWriteCmd(tt.device, tt.cmd, tt.params, r)
if !tt.expectErr && appErr != nil {
t.Errorf("%s expectErr:%v error:%v", tt.testName, tt.expectErr, appErr.Error())
return
Expand Down Expand Up @@ -386,9 +396,10 @@ func TestCommandAllHandler(t *testing.T) {
{"PartOfWriteCommandExecutionSuccess", "RandomValue_Uint8", `{"RandomValue_Uint8":"123"}`, "", methodSet, false},
{"PartOfWriteCommandExecutionFail", "error", `{"RandomValue_Uint8":"123"}`, "", methodSet, true},
}
r := newRequestWithContentType(clients.ContentTypeJSON)
for _, tt := range tests {
t.Run(tt.testName, func(t *testing.T) {
_, appErr := CommandAllHandler(tt.cmd, tt.body, tt.method, tt.queryParams)
_, appErr := CommandAllHandler(tt.cmd, tt.body, r)
if !tt.expectErr && appErr != nil {
t.Errorf("%s expectErr:%v error:%v", tt.testName, tt.expectErr, appErr.Error())
return
Expand Down Expand Up @@ -421,6 +432,8 @@ func TestCommandHandler(t *testing.T) {
t.Errorf("Fail to update device, error: %v", err)
}

r := newRequestWithContentType(clients.ContentTypeJSON)

tests := []struct {
testName string
vars map[string]string
Expand All @@ -441,7 +454,7 @@ func TestCommandHandler(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.testName, func(t *testing.T) {
_, appErr := CommandHandler(tt.vars, tt.body, tt.method, tt.queryParams)
_, appErr := CommandHandler(tt.vars, tt.body, r)
if !tt.expectErr && appErr != nil {
t.Errorf("%s expectErr:%v error:%v", tt.testName, tt.expectErr, appErr.Error())
return
Expand All @@ -453,3 +466,9 @@ func TestCommandHandler(t *testing.T) {
})
}
}

func newRequestWithContentType(contentType string) *http.Request {
req := httptest.NewRequest(http.MethodGet, "/", strings.NewReader("Test body"))
req.Header.Set(clients.ContentType, contentType)
return req
}
Loading

0 comments on commit c997980

Please sign in to comment.