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

Rename decrypted_payload, modify its format #173

Merged
merged 7 commits into from
Oct 27, 2020
Merged

Conversation

Patrik-Stas
Copy link
Contributor

@Patrik-Stas Patrik-Stas commented Oct 26, 2020

Previously, the format of single message within output vcx_messages_download was JSON with following structure

{
   "statusCode": <data>
   "payload": <data>
   "uid": <data>
   "refMsgId": <data>
   "deliveryDetails": <data>
   "decryptedMsg": <data>
}

And the content of decrypted_payload was string-encoded JSON with structure

{
	"@type" : {
      "name" : <data>
      "ver" : <data>
      "fmt" : <data>
	}
	"@msg": <Stringified JSON Aries message>
}

This PR deletes field "decrypted_payload", and introduces instead new field "decrypted_msg"

{
   "statusCode": <data>
   "payload": <data>
   "uid": <data>
   "refMsgId": <data>
   "deliveryDetails": <data>
   "decryptedPayload": <data>
}

whereas the decrypted_msg contains directly stringified Aries JSON message. Example:

{
    "statusCode": "MS-106",
    "payload": null,
    "uid": "c17da501-6c71-498c-a471-3409a3cb160d",
    "refMsgId": null,
    "deliveryDetails": [],
    "decryptedMsg": "{\"@id\":\"fcdc8068-97c4-459f-84ff-da9dd9089b42\",\"@type\":\"did:sov:BzCbsNYhMrjHiqZDTUASHg;spec/present-proof/1.0/ack\",\"status\":\"OK\",\"~thread\":{\"received_orders\":{},\"sender_order\":0,\"thid\":\"32e407a0-e32b-4870-9583-079daeb8414d\"}}"
  }

Signed-off-by: Patrik Stas patrik.stas@absa.africa

Signed-off-by: Patrik Stas patrik.stas@absa.africa

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
@lgtm-com
Copy link

lgtm-com bot commented Oct 26, 2020

This pull request fixes 1 alert when merging faf1b9a into 84ec0b6 - view on LGTM.com

fixed alerts:

  • 1 for Syntax error

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
@lgtm-com
Copy link

lgtm-com bot commented Oct 26, 2020

This pull request fixes 1 alert when merging bd9579d into 84ec0b6 - view on LGTM.com

fixed alerts:

  • 1 for Syntax error

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
@lgtm-com
Copy link

lgtm-com bot commented Oct 26, 2020

This pull request fixes 1 alert when merging dc3bcb8 into 84ec0b6 - view on LGTM.com

fixed alerts:

  • 1 for Syntax error

@codecov-io
Copy link

codecov-io commented Oct 26, 2020

Codecov Report

Merging #173 into master will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #173      +/-   ##
==========================================
+ Coverage   51.15%   51.16%   +0.01%     
==========================================
  Files         150      150              
  Lines       22899    22900       +1     
  Branches     6046     6047       +1     
==========================================
+ Hits        11713    11716       +3     
+ Misses       7580     7578       -2     
  Partials     3606     3606              
Flag Coverage Δ
#unittests 51.16% <0.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
libvcx/src/aries/handlers/connection/mod.rs 68.57% <ø> (ø)
libvcx/src/aries/mod.rs 8.12% <0.00%> (-0.03%) ⬇️
libvcx/src/connection.rs 62.25% <ø> (ø)
libvcx/src/messages/get_message.rs 33.77% <0.00%> (ø)
libvcx/src/lib.rs 33.48% <0.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84ec0b6...0f7c90d. Read the comment docs.

assert(payload["@id"])
assert(payload["@type"])

const msgs2 = await vcxAgent.serviceConnections.getMessagesV2(connectionId)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will extract the asserts to separate function and run them on the return value of getMessagesV2 as well

@@ -135,7 +135,16 @@ module.exports.createFaber = async function createFaber () {
return isValid
}

async function downloadReceivedMessages () {
logger.info('Faber is going to download messages wit')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the typo in my PR so please don't change to avoid conflicts

if filter_msg_type == msg_type {
return VcxAgencyMessage {
uid: message.uid,
decrypted_payload: msg_content.into(),
decrypted_payload: decrypted_msg.clone(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we are already doing breaking changes, should this field be rather called decrypted_msg?

@Patrik-Stas Patrik-Stas changed the title Keep breaking change on decrypted payload format Rename decrypted_payload, modify its format Oct 27, 2020
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
@lgtm-com
Copy link

lgtm-com bot commented Oct 27, 2020

This pull request fixes 1 alert when merging bdb0d30 into 84ec0b6 - view on LGTM.com

fixed alerts:

  • 1 for Syntax error

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
@lgtm-com
Copy link

lgtm-com bot commented Oct 27, 2020

This pull request fixes 1 alert when merging 0f7c90d into 84ec0b6 - view on LGTM.com

fixed alerts:

  • 1 for Syntax error

* Decrypt download_messages_v2 payload

Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>

* Add downloadReceivedMessagesV2 test to agent-core

Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
@Patrik-Stas Patrik-Stas requested a review from mirgee October 27, 2020 12:38
@mirgee mirgee merged commit 7f17f81 into master Oct 27, 2020
@mirgee mirgee deleted the keep-decryption-changes branch October 27, 2020 12:41
@lgtm-com
Copy link

lgtm-com bot commented Oct 27, 2020

This pull request fixes 1 alert when merging 000c58d into 84ec0b6 - view on LGTM.com

fixed alerts:

  • 1 for Syntax error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants