From 4d7ed080f3f3ed8b61b6d20ad0201d97aa64fb6d Mon Sep 17 00:00:00 2001 From: Mike Date: Mon, 5 Aug 2019 19:18:45 -0700 Subject: [PATCH] feature(query-params): Pass QueryParams through EdgeX to Device Services (#1571) Fix #1564 This pull request is an example/POC that @lenny-intel and I put together of how #1564 could be accomplished. This is only 1/2 of the equation as the device-sdk would need to be updated to handle the query parameters. The good thing with this is that it wouldn't be a breaking change as the device-sdk would simply ignore the query parameters that are sent on. Signed-off-by: Mike Johanson --- internal/core/command/device.go | 13 +++++----- internal/core/command/get.go | 16 ++++++++---- internal/core/command/get_test.go | 39 ++++++++++++++++++++++++----- internal/core/command/restDevice.go | 4 +-- 4 files changed, 53 insertions(+), 19 deletions(-) diff --git a/internal/core/command/device.go b/internal/core/command/device.go index fe07f0cccb..5070ac734b 100644 --- a/internal/core/command/device.go +++ b/internal/core/command/device.go @@ -23,7 +23,7 @@ import ( contract "github.com/edgexfoundry/go-mod-core-contracts/models" ) -func commandByDeviceID(deviceID string, commandID string, body string, isPutCommand bool, ctx context.Context) (string, int) { +func commandByDeviceID(deviceID string, commandID string, body string, queryParams string, isPutCommand bool, ctx context.Context) (string, int) { d, err := mdc.Device(deviceID, ctx) if err != nil { LoggingClient.Error(err.Error()) @@ -66,10 +66,10 @@ func commandByDeviceID(deviceID string, commandID string, body string, isPutComm return errMsg, http.StatusNotFound } - return commandByDevice(d, c, body, isPutCommand, ctx) + return commandByDevice(d, c, body, queryParams, isPutCommand, ctx) } -func commandByNames(dn string, cn string, body string, isPutCommand bool, ctx context.Context) (string, int) { +func commandByNames(dn string, cn string, body string, queryParams string, isPutCommand bool, ctx context.Context) (string, int) { d, err := mdc.DeviceForName(dn, ctx) if err != nil { LoggingClient.Error(err.Error()) @@ -89,16 +89,17 @@ func commandByNames(dn string, cn string, body string, isPutCommand bool, ctx co return err.Error(), http.StatusInternalServerError } } - return commandByDevice(d, command, body, isPutCommand, ctx) + + return commandByDevice(d, command, body, queryParams, isPutCommand, ctx) } -func commandByDevice(device contract.Device, command contract.Command, body string, isPutCommand bool, ctx context.Context) (string, int) { +func commandByDevice(device contract.Device, command contract.Command, body string, queryParams string, isPutCommand bool, ctx context.Context) (string, int) { var ex Executor var err error if isPutCommand { ex, err = NewPutCommand(device, command, body, ctx, &http.Client{}) } else { - ex, err = NewGetCommand(device, command, ctx, &http.Client{}) + ex, err = NewGetCommand(device, command, queryParams, ctx, &http.Client{}) } if err != nil { diff --git a/internal/core/command/get.go b/internal/core/command/get.go index f1e5427c80..3eaf1c0db9 100644 --- a/internal/core/command/get.go +++ b/internal/core/command/get.go @@ -15,17 +15,23 @@ package command import ( "context" + "net/http" + "net/url" + "strings" + "github.com/edgexfoundry/edgex-go/internal" "github.com/edgexfoundry/go-mod-core-contracts/clients" contract "github.com/edgexfoundry/go-mod-core-contracts/models" - "net/http" - "strings" ) // NewGetCommand creates and Executor which can be used to execute the GET related command. -func NewGetCommand(device contract.Device, command contract.Command, context context.Context, httpCaller internal.HttpCaller) (Executor, error) { - url := device.Service.Addressable.GetBaseURL() + strings.Replace(command.Get.Action.Path, DEVICEIDURLPARAM, device.Id, -1) - request, err := http.NewRequest(http.MethodGet, url, nil) +func NewGetCommand(device contract.Device, command contract.Command, queryParams string, context context.Context, httpCaller internal.HttpCaller) (Executor, error) { + urlResult := device.Service.Addressable.GetBaseURL() + strings.Replace(command.Get.Action.Path, DEVICEIDURLPARAM, device.Id, -1) + "?" + queryParams + validURL, err := url.ParseRequestURI(urlResult) + if err != nil { + return serviceCommand{}, err + } + request, err := http.NewRequest(http.MethodGet, validURL.String(), nil) if err != nil { return serviceCommand{}, err } diff --git a/internal/core/command/get_test.go b/internal/core/command/get_test.go index db3665f6d8..b1a9db69ea 100644 --- a/internal/core/command/get_test.go +++ b/internal/core/command/get_test.go @@ -15,9 +15,11 @@ package command import ( "context" + "strconv" + "testing" + "github.com/edgexfoundry/go-mod-core-contracts/clients" contract "github.com/edgexfoundry/go-mod-core-contracts/models" - "testing" ) const ( @@ -58,7 +60,7 @@ var testCommand = contract.Command{ func TestNewGetCommandWithCorrelationId(t *testing.T) { expectedCorrelationIDHeaderValue := "Testing" testContext := context.WithValue(context.Background(), clients.CorrelationHeader, expectedCorrelationIDHeaderValue) - getCommand, _ := NewGetCommand(testDevice, testCommand, testContext, nil) + getCommand, _ := NewGetCommand(testDevice, testCommand, "", testContext, nil) actualCorrelationIDHeaderValue := getCommand.(serviceCommand).Request.Header.Get(clients.CorrelationHeader) if actualCorrelationIDHeaderValue == "" { t.Errorf("The populated GetCommand's request should contain a correlation ID header value") @@ -69,8 +71,33 @@ func TestNewGetCommandWithCorrelationId(t *testing.T) { } } +func TestNewGetCommandWithQueryParams(t *testing.T) { + queryParams := "test=value1&test2=value2" + getCommand, _ := NewGetCommand(testDevice, testCommand, queryParams, context.Background(), nil) + req := getCommand.(serviceCommand).Request.URL + if req.Scheme != TestProtocol { + t.Errorf("Unexpected protocol") + } + expectedHost := TestAddress + ":" + strconv.Itoa(TestPort) + if req.Host != expectedHost { + t.Errorf("Unexpected host address and port") + } + if req.Path != testCommand.Get.Action.Path { + t.Errorf("Unexpected path") + } + if req.RawQuery != queryParams { + t.Errorf("Unexpected Raw Query Value") + } +} +func TestNewGetCommandWithMalformedQueryParams(t *testing.T) { + queryParams := "!@#$%" + _, err := NewGetCommand(testDevice, testCommand, queryParams, context.Background(), nil) + if err == nil { + t.Errorf("Expected error for malformed query parameters") + } +} func TestNewGetCommandNoCorrelationIDInContext(t *testing.T) { - getCommand, _ := NewGetCommand(testDevice, testCommand, context.Background(), nil) + getCommand, _ := NewGetCommand(testDevice, testCommand, "", context.Background(), nil) actualCorrelationIDHeaderValue := getCommand.(serviceCommand).Request.Header.Get(clients.CorrelationHeader) if actualCorrelationIDHeaderValue != "" { t.Errorf("No correlation ID should be specified") @@ -80,8 +107,8 @@ func TestNewGetCommandNoCorrelationIDInContext(t *testing.T) { func TestNewGetCommandInvalidBaseUrl(t *testing.T) { device := testDevice device.Service.Addressable.Address = "!@#$" - _, err := NewGetCommand(device, testCommand, context.Background(), nil) - if err != nil { - t.Errorf("The invalid URL error was not properly propegated to the caller") + _, err := NewGetCommand(device, testCommand, "", context.Background(), nil) + if err == nil { + t.Errorf("The invalid URL error was not properly propagated to the caller") } } diff --git a/internal/core/command/restDevice.go b/internal/core/command/restDevice.go index 7b5a94f21e..c9489e8dc0 100644 --- a/internal/core/command/restDevice.go +++ b/internal/core/command/restDevice.go @@ -42,7 +42,7 @@ func issueDeviceCommand(w http.ResponseWriter, r *http.Request, isPutCommand boo } ctx := r.Context() - body, status := commandByDeviceID(did, cid, string(b), isPutCommand, ctx) + body, status := commandByDeviceID(did, cid, string(b), r.URL.RawQuery, isPutCommand, ctx) if status != http.StatusOK { http.Error(w, body, status) } else { @@ -75,7 +75,7 @@ func issueDeviceCommandByNames(w http.ResponseWriter, r *http.Request, isPutComm LoggingClient.Error(err.Error()) return } - body, status := commandByNames(dn, cn, string(b), isPutCommand, ctx) + body, status := commandByNames(dn, cn, string(b), r.URL.RawQuery, isPutCommand, ctx) if status != http.StatusOK { http.Error(w, body, status)