Skip to content

Commit

Permalink
Address PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
kstribrnAmzn committed Jul 5, 2024
1 parent e3bf9e8 commit 0828b86
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 33 deletions.
26 changes: 13 additions & 13 deletions source/include/jobs.h
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ typedef enum
* @brief Structure for Jobs_UpdateMsg request parameters.
*
* @note For optional fields setting a pointer to NULL or
* the length to 0U will disable this field from being used
* the length to 0U will disable this field from being used.
*
*
* @note Optional fields include:
Expand All @@ -332,15 +332,15 @@ typedef enum
* * statusDetailsLength
*
* @note The status details must be a JSON formatted key-value
* paid.
* pair.
*/
typedef struct
{
JobCurrentStatus_t status; /**< Status to update the job to */
const char * expectedVersion; /**< Expected version, optional */
size_t expectedVersionLength; /**< Expected version length, optional */
const char * statusDetails; /**< JSON key-value pair, optional */
size_t statusDetailsLength; /**< JSON key-value pair length, optional */
JobCurrentStatus_t status; /**< Status to update the job to. */
const char * expectedVersion; /**< Expected version, optional. */
size_t expectedVersionLength; /**< Expected version length, optional. */
const char * statusDetails; /**< JSON key-value pair, optional. */
size_t statusDetailsLength; /**< JSON key-value pair length, optional. */
} JobsUpdateRequest_t;

/*-----------------------------------------------------------*/
Expand Down Expand Up @@ -867,19 +867,19 @@ JobsStatus_t Jobs_Update( char * buffer,
/**
* @brief Populate a message string for an UpdateJobExecution request.
*
* @param request A jobs update request structure
* @param buffer The buffer to be written to
* @param bufferSize the size of the buffer
* @param request A jobs update request structure.
* @param buffer The buffer to be written to.
* @param bufferSize the size of the buffer.
*
* @return 0 if write to buffer fails
* @return messageLength if the write is successful
* @return 0 if write to buffer fails.
* @return messageLength if the write is successful.
*
* <b>Example</b>
* @code{c}
*
* // The Following Example shows usage of the Jobs_UpdateMsg API to
* // generate a message string for the UpdateJobExecution API
* // of the AWS IoT Jobs Service
* // of the AWS IoT Jobs Service.
*
* const char * expectedVersion = "2";
* const chat * statusDetails = "{\"key\":\"value\"}";
Expand Down
31 changes: 11 additions & 20 deletions source/jobs.c
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,6 @@ static const char * const jobStatusString[] =
"REJECTED"
};

static const size_t jobStatusStringLengths[] =
{
CONST_STRLEN( "QUEUED" ),
CONST_STRLEN( "IN_PROGRESS" ),
CONST_STRLEN( "FAILED" ),
CONST_STRLEN( "SUCCEEDED" ),
CONST_STRLEN( "REJECTED" )
};

/**
* @brief Predicate returns true for a valid thing name or job ID character.
*
Expand Down Expand Up @@ -836,8 +827,8 @@ JobsStatus_t Jobs_Update( char * buffer,
* the Jobs_UpdateMsg. These optional fields, if provided, require
* additional buffer space.
*
* @param request A JobsUpdateRequest_t containing the optional fields
* @return size_t The buffer space required for the optional fields
* @param request A JobsUpdateRequest_t containing the optional fields.
* @return size_t The buffer space required for the optional fields.
*/
static size_t getOptionalFieldsLength( JobsUpdateRequest_t request )
{
Expand All @@ -860,21 +851,21 @@ static size_t getOptionalFieldsLength( JobsUpdateRequest_t request )
* @brief Get the total length of the required fields in the
* Jobs_UpdateMsg request.
*
* @param request A JobsUpdateRequest_t containing the optional fields
* @return size_t The buffer space required for the optional fields
* @param request A JobsUpdateRequest_t containing the optional fields.
* @return size_t The buffer space required for the optional fields.
*/
static size_t getRequiredFieldsLength( JobsUpdateRequest_t request )
{
return JOBS_API_STATUS_LENGTH + jobStatusStringLengths[ request.status ] + CONST_STRLEN( "\"}" );
return JOBS_API_STATUS_LENGTH + strlen(jobStatusString[ request.status ]) + CONST_STRLEN( "\"}" );
}

/**
* @brief Check non-null optional fields in the Jobs_UpdateMsg request
* for validity.
*
* @param request A JobsUpdateRequest_t containing the optional fields
* @return true Optional fields appear valid
* @return false Optional fields are invalid
* @param request A JobsUpdateRequest_t containing the optional fields.
* @return true Optional fields appear valid.
* @return false Optional fields are invalid.
*/
static bool areOptionalFieldsValid( JobsUpdateRequest_t request )
{
Expand All @@ -901,17 +892,17 @@ size_t Jobs_UpdateMsg( JobsUpdateRequest_t request,
if( !writeFailed )
{
( void ) strnAppend( buffer, &start, bufferSize, JOBS_API_STATUS, JOBS_API_STATUS_LENGTH );
( void ) strnAppend( buffer, &start, bufferSize, jobStatusString[ request.status ], jobStatusStringLengths[ request.status ] );
( void ) strnAppend( buffer, &start, bufferSize, jobStatusString[ request.status ], strlen(jobStatusString[ request.status ]) );
}

/* This is an optional field so do not fail if expected version is missing */
/* This is an optional field so do not fail if expected version is missing.*/
if( !writeFailed && ( request.expectedVersion != NULL ) && ( request.expectedVersionLength > 0U ) )
{
( void ) strnAppend( buffer, &start, bufferSize, JOBS_API_EXPECTED_VERSION, JOBS_API_EXPECTED_VERSION_LENGTH );
( void ) strnAppend( buffer, &start, bufferSize, request.expectedVersion, request.expectedVersionLength );
}

/* This is an optional field so do not fail if expected version is missing */
/* This is an optional field so do not fail if status details is missing.*/
if( !writeFailed && ( request.statusDetails != NULL ) && ( request.statusDetailsLength > 0U ) )
{
( void ) strnAppend( buffer, &start, bufferSize, JOBS_API_STATUS_DETAILS, JOBS_API_STATUS_DETAILS_LENGTH );
Expand Down
6 changes: 6 additions & 0 deletions test/unit-test/jobs_utest.c
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,7 @@ void test_getUpdateJobExecutionMsg_hasNullExpectedVersion( void )
size_t result = Jobs_UpdateMsg( request, buffer, TOPIC_BUFFER_SIZE );

TEST_ASSERT_EQUAL( 54U, result );
TEST_ASSERT_EQUAL_STRING("{\"status\":\"QUEUED\",\"statusDetails\":\"{\"key\": \"value\"}\"}", buffer);
}

void test_getUpdateJobExecutionMsg_hasZeroLengthExpectedVersion( void )
Expand All @@ -909,6 +910,7 @@ void test_getUpdateJobExecutionMsg_hasZeroLengthExpectedVersion( void )
size_t result = Jobs_UpdateMsg( request, buffer, TOPIC_BUFFER_SIZE );

TEST_ASSERT_EQUAL( 54U, result );
TEST_ASSERT_EQUAL_STRING("{\"status\":\"QUEUED\",\"statusDetails\":\"{\"key\": \"value\"}\"}", buffer);
}

void test_getUpdateJobExecutionMsg_hasNullStatusDetails( void )
Expand All @@ -926,6 +928,7 @@ void test_getUpdateJobExecutionMsg_hasNullStatusDetails( void )
size_t result = Jobs_UpdateMsg( request, buffer, TOPIC_BUFFER_SIZE );

TEST_ASSERT_EQUAL( 45U, result );
TEST_ASSERT_EQUAL_STRING("{\"status\":\"QUEUED\",\"expectedVersion\":\"1.0.1\"}", buffer);
}

void test_getUpdateJobExecutionMsg_hasZeroLengthStatusDetails( void )
Expand All @@ -943,6 +946,7 @@ void test_getUpdateJobExecutionMsg_hasZeroLengthStatusDetails( void )
size_t result = Jobs_UpdateMsg( request, buffer, TOPIC_BUFFER_SIZE );

TEST_ASSERT_EQUAL( 45U, result );
TEST_ASSERT_EQUAL_STRING("{\"status\":\"QUEUED\",\"expectedVersion\":\"1.0.1\"}", buffer);
}

void test_getUpdateJobExecutionMsg_hasMalformedStatusDetails( void )
Expand Down Expand Up @@ -1012,6 +1016,7 @@ void test_getUpdateJobExecutionMsg_hasAllValidParameters( void )
size_t result = Jobs_UpdateMsg( request, buffer, TOPIC_BUFFER_SIZE );

TEST_ASSERT_EQUAL( 80U, result );
TEST_ASSERT_EQUAL_STRING("{\"status\":\"QUEUED\",\"expectedVersion\":\"1.0.1\",\"statusDetails\":\"{\"key\": \"value\"}\"}", buffer);
}

void test_getUpdateJobExecutionMsg_hasRequiredValidParameters( void )
Expand All @@ -1029,4 +1034,5 @@ void test_getUpdateJobExecutionMsg_hasRequiredValidParameters( void )
size_t result = Jobs_UpdateMsg( request, buffer, TOPIC_BUFFER_SIZE );

TEST_ASSERT_EQUAL( 19U, result );
TEST_ASSERT_EQUAL_STRING("{\"status\":\"QUEUED\"}", buffer);
}

0 comments on commit 0828b86

Please sign in to comment.