Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle both JSON request bodies as well as CBOR request bodies. #488

Open
akramtexas opened this issue Apr 16, 2020 · 6 comments
Open

Handle both JSON request bodies as well as CBOR request bodies. #488

akramtexas opened this issue Apr 16, 2020 · 6 comments
Labels
1-low priority denoting isolated changes enhancement New feature or request

Comments

@akramtexas
Copy link
Contributor

  • Currently, we do not check the header value (i.e. whether ContentType is JSON or CBOR). Creating this functionality will be helpful and can be used in the future when such functionality possibly becomes a requisite.
  • This functionality has actually already been largely addressed, as part of some CBOR-related work done in the edgex-go repo, so there is no need to start from scratch.
  • More details can be found on the following feature branch: Functionality largely addressed.
  • Mentioning the following snippet for reference:
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
  }

  if len(paramMap) == 0 {
    err = fmt.Errorf("no parameters specified")
    return
  }
  return
}
@cloudxxx8
Copy link
Member

@FelixTing could you please verify this issue? thanks

@FelixTing
Copy link
Member

Hi @cloudxxx8, yes, I tested a get binary command with device-virtual and the response header is correct. And we do need to handle request bodies according to the ContentType value in header. https://github.com/edgexfoundry/device-sdk-go/blob/master/internal/handler/command.go#L414

@cloudxxx8
Copy link
Member

The related document has been addressed in edgexfoundry/edgex-docs#118

@cloudxxx8 cloudxxx8 assigned chr1shung and FelixTing and unassigned FelixTing Apr 20, 2020
@cloudxxx8 cloudxxx8 added the hanoi Hanoi release label Jun 29, 2020
@cloudxxx8 cloudxxx8 added this to the Hanoi milestone Jun 29, 2020
@chr1shung chr1shung added ireland and removed hanoi Hanoi release labels Oct 8, 2020
@chr1shung
Copy link

v2 binary handling is under discussion

@cloudxxx8
Copy link
Member

@iain-anderson please add this issue to today's meeting agenda. we might need to decide whether we want to handle binary set command in this release.

@iain-anderson
Copy link
Member

At the WG on 19th April, this is not priority for 2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-low priority denoting isolated changes enhancement New feature or request
Projects
Status: Icebox
Development

No branches or pull requests

5 participants