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

UpdateMsg Format #107

Closed
TheCrazyKing opened this issue Jul 29, 2024 · 7 comments
Closed

UpdateMsg Format #107

TheCrazyKing opened this issue Jul 29, 2024 · 7 comments

Comments

@TheCrazyKing
Copy link

Hi,

It seems that when following the example to create a update msg yields something like

"{"status":"SUCCEEDED","statusDetails":"{"key":"value"}"}}

Now, I don't know how you expect the MQTT API to be like, but in my case having a JSON content enclosed in quotes ("{"key":"val"}") is not working. I think it is safe to assume that job details will always be formatted as JSON and hence these quotes should be omitted.

If you don't agree I'll just use a local patch, so let me know :)

All the best

Ref:

 * const char * expectedVersion = "2";
 * const chat * statusDetails = "{\"key\":\"value\"}";
 * char messageBuffer[ UPDATE_JOB_MSG_LENGTH ]  = {0};
 * size_t messageLength = 0U;
 *
 * JobsUpdateRequest_t request;
 * request.status = Succeeded;
 * request.expectedVersion = expectedVersion;
 * request.expectedVersionLength = ( sizeof( expectedVersion ) - 1U );
 * request.statusDetails = statusDetails;
 * request.statusDetailsLength = ( sizeof( statusDetails ) - 1U );
 *
 * messageLength = Jobs_UpdateMsg( request
 *                                 messageBuffer,
 *                                 UPDATE_JOB_MSG_LENGTH );
 *
 * if (messageBufferLength > 0 )
 * {
 *     // The message string of length, messageLength, has been
 *     // generated in the buffer, messageBuffer, for the UpdateJobExecution API
 *     // Publish this message to the topic generated by Jobs_Update using an
 *     // MQTT client of your choice.
 * }
 * @endcode
 */
/* @[declare_jobs_updatemsg] */
size_t Jobs_UpdateMsg( JobsUpdateRequest_t request,
                       char * buffer,
                       size_t bufferSize );
@ActoryOu
Copy link
Member

Hello @TheCrazyKing,
Thanks for bringing this into our attension! I've verified that example and it's also failure in my case.
I'll create a PR to fix that.

The problem of this example is not the quote, and the statusDetails is also expected to be JSON format (checked by areOptionalFieldsValid()). Note that the expected result would be: {"status":"SUCCEEDED","expectedVersion":"2","statusDetails":"{"key":"value"}"}.

Thank you.

@TheCrazyKing
Copy link
Author

TheCrazyKing commented Jul 30, 2024

Ok! I saw your PR and it indeed corrects a bug. Now, I had just this question regarding the quotes if I may:
when publishing (with the AWS IoT MQTT test client) to the jobs update topic, the following content throws an error ({ "code": "InvalidJson", "message": "Unexpected character ('p' (code 112)): was expecting comma to separate Object entries at [Source: (iot.laser.tjm.handler.utils.DeviceMessageParser$1); line: 1, column: 68]" }):

{
 "status":"IN_PROGRESS",
 "statusDetails": "{"progress":"testerQuotes"}"
}

but the following message format is working:

{
 "status":"IN_PROGRESS",
 "statusDetails": {"progress":"testerQuotes"}
}

Is it still compliant with the expected result {"status":"SUCCEEDED","expectedVersion":"2","statusDetails":"{"key":"value"}"} you mentioned ? Or am I missing something here ?

PS: the message format

{"status":"IN_PROGRESS","statusDetails":"{'progress':'testerQuotes'}"}

gives this error "code": "InvalidRequest","message": "Value at statusDetails cannot be a STRING for topic $aws/XXX

@aggarg
Copy link
Member

aggarg commented Jul 30, 2024

Is it still compliant with the expected result {"status":"SUCCEEDED","expectedVersion":"2","statusDetails":"{"key":"value"}"} you mentioned ? Or am I missing something here ?

This is not a valid JSON. You can check that using any JSON validator - https://jsonformatter.org/json-pretty-print.

@ActoryOu
Copy link
Member

Hi @TheCrazyKing,
"{"progress":"testerQuotes"}" is not a valid JSON, as @aggarg said.
When we declare a char string like const chat statusDetails[] = "{\"key\":\"value\"}";, the string is actually {"key":"value"}. No quote at the begining and no quote at the end.

Thank you.

@ActoryOu
Copy link
Member

Closing because it's resolved.

@TheCrazyKing
Copy link
Author

TheCrazyKing commented Jul 31, 2024

@ActoryOu
Ok but the SDK (JobsUpdateMsg) will enclose the status details JSON with quotes:

#define JOBS_API_STATUS_DETAILS "\",\"statusDetails\":\""
and at the end of the function:
( void ) strnAppend( buffer, &start, bufferSize, "\"}", ( CONST_STRLEN( "\"}" ) ) );

I would apply a patch like this one:

diff --git a/source/include/jobs.h b/source/include/jobs.h
index 0939e54..ec3864d 100644
--- a/source/include/jobs.h
+++ b/source/include/jobs.h
@@ -150,7 +150,7 @@
 #define JOBS_API_EXPECTED_VERSION           "\",\"expectedVersion\":\""
 #define JOBS_API_EXPECTED_VERSION_LENGTH    ( sizeof( JOBS_API_EXPECTED_VERSION ) - 1U )
 
-#define JOBS_API_STATUS_DETAILS             "\",\"statusDetails\":\""
+#define JOBS_API_STATUS_DETAILS             "\",\"statusDetails\":"
 #define JOBS_API_STATUS_DETAILS_LENGTH      ( sizeof( JOBS_API_STATUS_DETAILS ) - 1U )
 
 #define JOBS_API_COMMON_LENGTH( thingNameLength ) \
diff --git a/source/jobs.c b/source/jobs.c
index bd15740..89c4210 100644
--- a/source/jobs.c
+++ b/source/jobs.c
@@ -907,13 +907,17 @@ size_t Jobs_UpdateMsg( JobsUpdateRequest_t request,
     {
         ( void ) strnAppend( buffer, &start, bufferSize, JOBS_API_STATUS_DETAILS, JOBS_API_STATUS_DETAILS_LENGTH );
         ( void ) strnAppend( buffer, &start, bufferSize, request.statusDetails, request.statusDetailsLength );
-    }
 
-    if( !writeFailed )
-    {
-        ( void ) strnAppend( buffer, &start, bufferSize, "\"}", ( CONST_STRLEN( "\"}" ) ) );
+        ( void ) strnAppend( buffer, &start, bufferSize, "}", ( CONST_STRLEN( "}" ) ) );
+    }else{
+
+        if( !writeFailed )
+        {
+            ( void ) strnAppend( buffer, &start, bufferSize, "\"}", ( CONST_STRLEN( "\"}" ) ) );
+        }
     }
 
+
     return start;
 }
 

If you append \" after the job status details, there will be a quote after the JSON status detail.

If you do not agree, I'll use the patch only locally.

All the best

aggarg added a commit to aggarg/Jobs-for-AWS-IoT-embedded-sdk that referenced this issue Jul 31, 2024
Jobs_UpdateMsg was putting double quotes around the `statusDetails`
value resulting in the following invalid JSON:
```
{
  "status": "QUEUED",
  "statusDetails": "{
    "key": "value"
  }"
}
```

This change removes the unnecessary double quotes to generate a valid
JSON:
```
{
  "status": "QUEUED",
  "statusDetails": {
    "key": "value"
  }
}
```

This was reported here - aws#107

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
@aggarg
Copy link
Member

aggarg commented Jul 31, 2024

You are exactly right. This PR addresses the issue - #109.

Thank you for reporting it.

AniruddhaKanhere pushed a commit that referenced this issue Jul 31, 2024
Jobs_UpdateMsg was putting double quotes around the `statusDetails`
value resulting in the following invalid JSON:
```
{
  "status": "QUEUED",
  "statusDetails": "{
    "key": "value"
  }"
}
```

This change removes the unnecessary double quotes to generate a valid
JSON:
```
{
  "status": "QUEUED",
  "statusDetails": {
    "key": "value"
  }
}
```

This was reported here - #107

Signed-off-by: Gaurav Aggarwal <aggarg@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants