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

Align OutOfBandManager.receive_invitation with other connection managers #1382

Conversation

chumbert
Copy link
Contributor

@chumbert chumbert commented Sep 1, 2021

Refactoring step before proposing changes in mediation connection
setup.

  • Align OutOfBandManager.receive_invitation signature with the
    ConnectionManager.receive_invitation and
    DIDXManager.receive_invitation

    • rename first parameter to invitation
    • change return type to ConnRecord and leave serialization work
      for users of the manager
  • Serialize the received ConnRecord in
    protocols.out_of_band.v1_0.routes

  • Adapt related tests. assert ConnRecord.deserialize(conn_rec) is dubious as a test assertion but boiled down to assert conn_rec is not None.

  • Use this refactoring to reduce branching in mediator connection
    handling in conductor.py.

Signed-off-by: Clément Humbert clement.humbert@gmail.com

@chumbert chumbert force-pushed the oob-connection-manager-api-alignment branch from 813d05e to 76fd6bd Compare September 2, 2021 06:04
setup.

* Align `OutOfBandManager.receive_invitation` signature with the
  `ConnectionManager.receive_invitation` and
  `DIDXManager.receive_invitation`
    * rename first parameter to `invitation`
    * change return type to `ConnRecord` and leave serialization work
      for users of the manager

* Serialize the received `ConnRecord` in
  `protocols.out_of_band.v1_0.routes`

* Adapt related tests. `assert
  ConnRecord.deserialize(conn_rec)` is dubious as a test assertion but boiled down to `assert conn_rec
  is not None`.

* Use this refactoring to reduce branching in mediator connection
  handling in `conductor.py`.

Signed-off-by: Clément Humbert <clement.humbert@gmail.com>
@chumbert chumbert force-pushed the oob-connection-manager-api-alignment branch from 76fd6bd to f12df9d Compare September 2, 2021 07:12
Copy link
Contributor

@dbluhm dbluhm left a comment

Choose a reason for hiding this comment

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

LGTM!

@codecov-commenter
Copy link

Codecov Report

Merging #1382 (ee301d7) into main (53878e7) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1382      +/-   ##
==========================================
- Coverage   95.28%   95.27%   -0.01%     
==========================================
  Files         476      476              
  Lines       29034    29025       -9     
==========================================
- Hits        27664    27655       -9     
  Misses       1370     1370              

@andrewwhitehead andrewwhitehead merged commit b8ca1da into openwallet-foundation:main Sep 2, 2021
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

Successfully merging this pull request may close these issues.

4 participants