feat: messagebox

[m00sey]

Title:
proposed implementation for a messagebox feature

Description:
implements #811 service layer

Summary:

Introduction of a messagepickup service for storing messages until and edge agent collects.

I'd like to credit @pfeairheller for the original implementation.

Since this violates the ~500 line PR rule I wanted to get it out early for discussion.
I am still evaluating what BDD tests might be needed and will mark this current PR as draft.

I am making a corresponding PR to aries-rfcs, to update the type values of messages.
hyperledger/aries-rfcs#500

[codecov]

Codecov Report

Merging #1926 into master will decrease coverage by 0.09%.
The diff coverage is 85.61%.

@@            Coverage Diff             @@
##           master    #1926      +/-   ##
==========================================
- Coverage   90.97%   90.88%   -0.10%     
==========================================
  Files         218      220       +2     
  Lines       16150    16435     +285     
==========================================
+ Hits        14693    14937     +244     
- Misses        856      882      +26     
- Partials      601      616      +15     

Impacted Files	Coverage Δ
pkg/didcomm/protocol/messagepickup/service.go	85.55% <85.55%> (ø)
pkg/didcomm/protocol/messagepickup/lockbox.go	86.66% <86.66%> (ø)

---

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 cbafe55...5fe6569. Read the comment docs.

[llorllale]

@m00sey I think it's headed in the right direction. I am afraid of the coupled getInbox/putInbox pairs in AddMessage and pullMessages, especially the latter. Looks dangerous and prone to data loss. I think a lock on theirDID is needed.

[m00sey]

@m00sey I think it's headed in the right direction. I am afraid of the coupled getInbox/putInbox pairs in AddMessage and pullMessages, especially the latter. Looks dangerous and prone to data loss. I think a lock on theirDID is needed.

I added additional locks around each theirDID and a map to manage those mutex.

I am looking into why it thinks I have touched the additional six files, I don't see them in my Files Changed list and make checks passes locally.

[llorllale]

@m00sey

@m00sey I think it's headed in the right direction. I am afraid of the coupled getInbox/putInbox pairs in AddMessage and pullMessages, especially the latter. Looks dangerous and prone to data loss. I think a lock on theirDID is needed.

I added additional locks around each theirDID and a map to manage those mutex.

I am looking into why it thinks I have touched the additional six files, I don't see them in my Files Changed list and make checks passes locally.

Sorry for the late reply. I think the code looks correct, but now I regret having suggested to use locks. The code gets so complicated.

But don't worry - we can merge this because it looks like it works without data loss. Just need to pass the checks and I think we can merge it.

[m00sey]

Agreed locking has increased the complexity, open to other thoughts on how to block access to avoid data loss.

[llorllale]

@m00sey please rebase on master

[m00sey]

@m00sey please rebase on master

Hm, so I rebased using git rebase=true upstream master where upstream is [email protected]:hyperledger/aries-framework-go.git resolved the conflicts and pushed, I wasn't expecting the additional changes to be tracked in my PR...

Is there a different/preferred way? Apologies.

I'll work on the coverage today, I thought I had scraped past 85%, I am guessing it's a lines vs statements difference?

┌─[±][issue-811 → origin ✓][messagepickup][]
└─▪ go test -coverprofile=coverage.out && go tool cover -html=coverage.out
PASS
coverage: 87.0% of statements
ok  	github.com/hyperledger/aries-framework-go/pkg/didcomm/protocol/messagepickup	0.004s

[llorllale]

I'll work on the coverage today, I thought I had scraped past 85%, I am guessing it's a lines vs statements difference?

┌─[±][issue-811 → origin ✓][messagepickup][]
└─▪ go test -coverprofile=coverage.out && go tool cover -html=coverage.out
PASS
coverage: 87.0% of statements
ok  	github.com/hyperledger/aries-framework-go/pkg/didcomm/protocol/messagepickup	0.004s

It's the coverage on the patch (delta) that's holding this back. See the report here.

[rolsonquadras]

Title:
proposed implementation for a messagebox feature

Description:
closes #811

@m00sey Thanks for the pull request.

One minor note - This PR doesn't close #811 as this only implements service layer. We would need to add following to make this feature complete.

• Store the messages when registered agent is unavailable
• Integrate with framework
• Client Layer
• BDD tests

I updated the issue with these details - #811 (comment)

[m00sey]

@rolsonquadras understood, I was overzealous. I will remove that from the description in order to keep this PR moving. I plan to address the subsequent parts in future PRs.

[llorllale]

@m00sey thanks!