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

feat: use computed tags for records #313

Merged
merged 5 commits into from
Jun 23, 2021

Conversation

TimoGlastra
Copy link
Contributor

@TimoGlastra TimoGlastra commented Jun 9, 2021

This allows to use computed tags, i.e. tags that are automatically set when retrieving the tags/storing the record.

The benefit is that it becomes a lot easier to add tags that need to stay in sync with the record values. For example records can now be queried by state, which is automatically updated when the record is saved.

In addition it partly fixes #290 by supporting boolean values and transforming those from and to store when storing/retrieving records. I'm not sure if we want other types supported? If so I'll address that in a separate PR

Also fixes #258

@TimoGlastra TimoGlastra requested a review from a team as a code owner June 9, 2021 11:37
@codecov-commenter
Copy link

codecov-commenter commented Jun 9, 2021

Codecov Report

Merging #313 (32ba89d) into main (6a26337) will decrease coverage by 0.01%.
The diff coverage is 90.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #313      +/-   ##
==========================================
- Coverage   88.39%   88.38%   -0.02%     
==========================================
  Files         223      225       +2     
  Lines        4043     4088      +45     
  Branches      447      455       +8     
==========================================
+ Hits         3574     3613      +39     
- Misses        469      475       +6     
Impacted Files Coverage Δ
src/modules/basic-messages/BasicMessageEvents.ts 100.00% <ø> (ø)
src/storage/BaseRecord.ts 61.11% <45.45%> (-28.89%) ⬇️
.../modules/credentials/services/CredentialService.ts 88.17% <50.00%> (ø)
src/modules/proofs/services/ProofService.ts 85.53% <50.00%> (ø)
src/modules/basic-messages/BasicMessageRole.ts 100.00% <100.00%> (ø)
src/modules/basic-messages/index.ts 100.00% <100.00%> (ø)
...es/basic-messages/repository/BasicMessageRecord.ts 100.00% <100.00%> (ø)
...les/basic-messages/services/BasicMessageService.ts 90.90% <100.00%> (-0.76%) ⬇️
...modules/connections/repository/ConnectionRecord.ts 96.66% <100.00%> (+0.30%) ⬆️
.../modules/connections/services/ConnectionService.ts 99.30% <100.00%> (-0.01%) ⬇️
... and 9 more

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 6a26337...32ba89d. Read the comment docs.

src/storage/BaseRecord.ts Outdated Show resolved Hide resolved
src/storage/BaseRecord.ts Outdated Show resolved Hide resolved
src/storage/BaseRecord.ts Outdated Show resolved Hide resolved
src/storage/BaseRecord.ts Show resolved Hide resolved
@TimoGlastra TimoGlastra force-pushed the computed-tags branch 2 times, most recently from 5ac0afb to b1b8be4 Compare June 21, 2021 09:31

export type Tags<DefaultTags, CustomTags extends TagsBase> = CustomTags & DefaultTags

export abstract class BaseRecord<DefaultTags = Record<string, unknown>, CustomTags extends TagsBase = TagsBase> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I’m sorry, I don’t want to bother with nitpicking, but I try to understand the typings here. Bear with me, please :)

Does it mean that DefaultTags allowing the data structures that are prohibited by TagsBase?

Is the goal that CustomTags cannot have number as a key or value but DefaultTags can?

I would assume that DefaultTags should have the same type as TagsBase. Then, maybe, we could simplified it to this:

export type TagValue = string | boolean | undefined
export type Tags = {
  [key: string]: TagValue
  [key: number]: never
}

export type RecordTags<DefaultTags, Tags> = Tags & DefaultTags

export abstract class BaseRecord<DefaultTags = Tags, CustomTags = Tags> {

Copy link
Contributor

@jakubkoci jakubkoci Jun 23, 2021

Choose a reason for hiding this comment

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

Of course, we can merge this if you want, I know it's been here for a while. I'm just curious if there is a way to simplify it a bit because it took me some time to understand what exactly we want to achieve with those types. I understand theres is some inherent complexity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't mind the nitpicking :)

You're totally right. I was struggling a bit with the types and wanted to indicate that by default he DefaultTags is an empty object. But with Record<string, unknown> that isn't any more clear. I changed it so both extend the TagsBase type now.

Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
@TimoGlastra
Copy link
Contributor Author

I think all issues should be resolved now @jakubkoci

Copy link
Contributor

@jakubkoci jakubkoci left a comment

Choose a reason for hiding this comment

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

Nice 👍

@TimoGlastra TimoGlastra merged commit 4e9a48b into openwallet-foundation:main Jun 23, 2021
@TimoGlastra TimoGlastra deleted the computed-tags branch June 23, 2021 16:05
burdettadam pushed a commit to atb-leap/aries-framework-javascript that referenced this pull request Jun 30, 2021
BREAKING CHANGE: Tags on a record can now be accessed using the `getTags()` method. Records should be updated with this method and return the properties from the record to include in the tags.

Signed-off-by: Timo Glastra <timo@animo.id>
burdettadam added a commit to atb-leap/aries-framework-javascript that referenced this pull request Jul 12, 2021
commit 92a6c01
Merge: b292c34 624a8d9
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Mon Jul 12 08:39:11 2021 -0600

    Merge branch 'TimoGlastra-0211-3' into feature/0211-route-coordination

commit 624a8d9
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Mon Jul 12 08:37:30 2021 -0600

    in memory message repo for sender tests

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit a1a36d3
Merge: b292c34 d7294ca
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Mon Jul 12 07:45:49 2021 -0600

    merge draft

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit b292c34
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Mon Jul 12 07:00:40 2021 -0600

    formatting

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit d7294ca
Author: Timo Glastra <timo@animo.id>
Date:   Mon Jul 12 12:50:19 2021 +0200

    test: full mediation flow

    Signed-off-by: Timo Glastra <timo@animo.id>

commit 3141098
Author: Timo Glastra <timo@animo.id>
Date:   Mon Jul 12 10:15:49 2021 +0200

    feat: pickup messages from mediator

    Signed-off-by: Timo Glastra <timo@animo.id>

commit 0d021f0
Author: Timo Glastra <timo@animo.id>
Date:   Mon Jul 12 09:41:32 2021 +0200

    refactor: use new query syntax for arrays

    Signed-off-by: Timo Glastra <timo@animo.id>

commit 1d5636c
Author: Timo Glastra <timo@animo.id>
Date:   Mon Jul 12 09:39:32 2021 +0200

    feat: add mediator pickup strategy none

    Signed-off-by: Timo Glastra <timo@animo.id>

commit cacf20f
Merge: 906d1ae fb28935
Author: Timo Glastra <timo@animo.id>
Date:   Mon Jul 12 09:31:09 2021 +0200

    Merge remote-tracking branch 'upstream/main' into 0211-3

commit 906d1ae
Author: Timo Glastra <timo@animo.id>
Date:   Mon Jul 12 01:58:10 2021 +0200

    style: small cleanup

    Signed-off-by: Timo Glastra <timo@animo.id>

commit dd6402d
Author: Timo Glastra <timo@animo.id>
Date:   Mon Jul 12 01:45:56 2021 +0200

    fix: e2e.tests.ts  working

    Signed-off-by: Timo Glastra <timo@animo.id>

commit dbc9b3f
Author: Timo Glastra <timo@animo.id>
Date:   Mon Jul 12 01:00:58 2021 +0200

    fix: agents.test.ts working

    Signed-off-by: Timo Glastra <timo@animo.id>

commit a3d46da
Author: Timo Glastra <timo@animo.id>
Date:   Mon Jul 12 00:23:39 2021 +0200

    feat: add mediator pickup strategy

    Signed-off-by: Timo Glastra <timo@animo.id>

commit 4d5493e
Author: Timo Glastra <timo@animo.id>
Date:   Mon Jul 12 00:07:52 2021 +0200

    fix: incorrect forward logic

    Signed-off-by: Timo Glastra <timo@animo.id>

commit 57e85aa
Author: Timo Glastra <timo@animo.id>
Date:   Sun Jul 11 23:31:11 2021 +0200

    fix: type errors

    Signed-off-by: Timo Glastra <timo@animo.id>

commit 8741210
Author: Timo Glastra <timo@animo.id>
Date:   Sun Jul 11 23:26:53 2021 +0200

    refactor: handle polling inside the framework

    Signed-off-by: Timo Glastra <timo@animo.id>

commit e75414c
Merge: 3c3a120 ea91559
Author: Timo Glastra <timo@animo.id>
Date:   Sat Jul 10 18:07:45 2021 +0200

    Merge remote-tracking branch 'upstream/main' into 0211-3

commit 3c3a120
Author: Timo Glastra <timo@animo.id>
Date:   Sat Jul 10 18:04:54 2021 +0200

    merge main

    Signed-off-by: Timo Glastra <timo@animo.id>

commit e6d70c1
Author: Timo Glastra <timo@animo.id>
Date:   Sat Jul 10 16:40:42 2021 +0200

    refactor: more mediation cleanup

    Signed-off-by: Timo Glastra <timo@animo.id>

commit d308ffc
Author: Timo Glastra <timo@animo.id>
Date:   Sat Jul 10 16:36:21 2021 +0200

    docs: update docs to newest API

    Signed-off-by: Timo Glastra <timo@animo.id>

commit 4459789
Merge: fb19af9 e7a56a8
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Fri Jul 9 15:11:16 2021 -0600

    Merge pull request #13 from TimoGlastra/0211

    refactor: add assert connection on inbound message

commit e7a56a8
Merge: 0052b79 fb19af9
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Fri Jul 9 15:10:34 2021 -0600

    Merge branch 'feature/0211-route-coordination' into 0211

commit fb19af9
Merge: 8b09cc8 a46bd21
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Fri Jul 9 14:05:05 2021 -0600

    Merge pull request #14 from TimoGlastra/0211-2

    refactor: mediator recipient cleanup

commit 8b09cc8
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Fri Jul 9 14:02:47 2021 -0600

    updated connections interface

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit a46bd21
Author: Timo Glastra <timo@animo.id>
Date:   Fri Jul 9 21:40:51 2021 +0200

    refactor: mediator module and service

    Signed-off-by: Timo Glastra <timo@animo.id>

commit d29387c
Author: Timo Glastra <timo@animo.id>
Date:   Fri Jul 9 19:31:28 2021 +0200

    refactor: mediator recipient cleanup

    Signed-off-by: Timo Glastra <timo@animo.id>

commit 0052b79
Author: Timo Glastra <timo@animo.id>
Date:   Fri Jul 9 18:43:14 2021 +0200

    fix: remove unneeded injection symbols

    Signed-off-by: Timo Glastra <timo@animo.id>

commit 3f73727
Author: Timo Glastra <timo@animo.id>
Date:   Fri Jul 9 18:31:47 2021 +0200

    refactor: replace waitForEvent with observables

    Signed-off-by: Timo Glastra <timo@animo.id>

commit 88faae8
Author: Timo Glastra <timo@animo.id>
Date:   Fri Jul 9 17:55:34 2021 +0200

    refactor: add assert connection on inbound message

    Signed-off-by: Timo Glastra <timo@animo.id>

commit c0c9df1
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Fri Jul 9 01:52:39 2021 -0600

    transport injection bug

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit 319690d
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Fri Jul 9 01:49:42 2021 -0600

    moved update keylist into mediator

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit 6731aca
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 8 23:12:38 2021 -0600

    path fix for lint

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit 8b2f10a
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 8 22:46:53 2021 -0600

    formatting

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit 315ca8c
Merge: b5ae8f1 b3213b9
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 8 22:36:26 2021 -0600

    Merge branch 'feature/0211-route-coordination' of github.com:atb-leap/aries-framework-javascript into feature/0211-route-coordination

commit b5ae8f1
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 8 22:36:14 2021 -0600

    moved mediation ping

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit b3213b9
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 8 16:29:10 2021 -0600

    Update src/modules/routing/services/RecipientService.ts

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

    Co-authored-by: jakubkoci <jakub.koci@gmail.com>

commit 3ccc0e1
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 8 16:28:54 2021 -0600

    Update src/modules/routing/services/RecipientService.ts

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

    Co-authored-by: jakubkoci <jakub.koci@gmail.com>

commit ca4a70b
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 1 19:36:22 2021 -0600

    deb injection for inbound transporter

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit 11448bc
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 1 18:36:25 2021 -0600

    small changes after merge

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit a81981b
Merge: 39117c2 ddaec34
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 1 18:30:04 2021 -0600

    Merge branch 'feature/0211-route-coordination' of github.com:atb-leap/aries-framework-javascript into feature/0211-route-coordination

commit 39117c2
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 1 17:40:48 2021 -0600

    return route in message sender

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit ddaec34
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 1 11:05:18 2021 -0600

    Update src/modules/connections/services/ConnectionService.ts

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

    Co-authored-by: Timo Glastra <timo@animo.id>

commit a332eac
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 1 11:04:11 2021 -0600

    Update src/modules/connections/services/ConnectionService.ts

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

    Co-authored-by: Timo Glastra <timo@animo.id>

commit fda92c5
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 1 11:03:56 2021 -0600

    Update src/modules/connections/services/ConnectionService.ts

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

    Co-authored-by: Timo Glastra <timo@animo.id>

commit 250ce52
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 1 11:03:14 2021 -0600

    Update src/modules/connections/services/ConnectionService.ts

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

    Co-authored-by: Timo Glastra <timo@animo.id>

commit 66d4d87
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 1 01:46:56 2021 -0600

    formatting

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit 8fe9dd9
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Thu Jul 1 01:45:06 2021 -0600

    a few improvements

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit 1ca3171
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Wed Jun 30 15:40:12 2021 -0600

    lost tags commit

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit 6aade61
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Wed Jun 30 14:49:20 2021 -0600

    formatting

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit ccf6bcf
Author: Timo Glastra <timo@animo.id>
Date:   Wed Jun 23 18:05:30 2021 +0200

    feat: use computed tags for records (openwallet-foundation#313)

    BREAKING CHANGE: Tags on a record can now be accessed using the `getTags()` method. Records should be updated with this method and return the properties from the record to include in the tags.

    Signed-off-by: Timo Glastra <timo@animo.id>

commit 286f48d
Author: Adam Burdett <burdettadam@gmail.com>
Date:   Mon Jun 28 14:47:26 2021 -0600

    feature/0211-route-coordination

    Signed-off-by: Adam Burdett <burdettadam@gmail.com>

commit 0c4206a
Author: Timo Glastra <timo@animo.id>
Date:   Wed Jun 30 17:48:22 2021 +0200

    feat: add inbound message queue (openwallet-foundation#339)

    * feat: add inbound message queue
    * refactor: use observables for serial processing

    Signed-off-by: Timo Glastra <timo@animo.id>

Signed-off-by: Adam Burdett <burdettadam@gmail.com>
Co-authored-by: David Clawson <david.clawson@gmail.com>
Co-authored-by: Ariel Gentile <gentilester@gmail.com>
Co-authored-by: Daniel Bluhm <dbluhm@pm.me>
Co-authored-by: Patrick Kenyon <treek.kenyon@gmail.com>
Co-authored-by: James Ebert <jamesebert.k@gmail.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

Successfully merging this pull request may close these issues.

support other types than strings for wallet record tags Basic messages improvements
3 participants