feat: connection type

[KolbyRKunz]

Added a connection type enum to the connection record so that you can filter the connections based on type.

[KolbyRKunz]

We went with putting the connection type in as a tag so that you can query a specific connection type, as was discussed in a previous working group call.

[codecov-commenter]

Codecov Report

Merging #994 (e91aa01) into main (8efade5) will increase coverage by 0.00%.
The diff coverage is 84.61%.

@@           Coverage Diff           @@
##             main     #994   +/-   ##
=======================================
  Coverage   88.70%   88.71%           
=======================================
  Files         521      522    +1     
  Lines       12130    12156   +26     
  Branches     2003     1913   -90     
=======================================
+ Hits        10760    10784   +24     
- Misses       1308     1368   +60     
+ Partials       62        4   -58     

Impacted Files	Coverage Δ
...modules/connections/repository/ConnectionRecord.ts	95.65% <ø> (ø)
.../modules/connections/services/ConnectionService.ts	87.09% <0.00%> (-0.71%)	⬇️
.../core/src/modules/connections/ConnectionsModule.ts	71.03% <88.88%> (+2.53%)	⬆️
...e/src/modules/connections/models/ConnectionType.ts	100.00% <100.00%> (ø)
...kages/core/src/modules/connections/models/index.ts	100.00% <100.00%> (ø)
...ules/routing/services/MediationRecipientService.ts	85.63% <100.00%> (+0.24%)	⬆️
packages/node/src/NodeFileSystem.ts	37.14% <0.00%> (ø)
packages/core/src/crypto/JwsService.ts	90.90% <0.00%> (ø)
packages/core/src/agent/MessageSender.ts	85.88% <0.00%> (ø)
packages/core/src/agent/MessageReceiver.ts	80.76% <0.00%> (ø)
... and 26 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[TimoGlastra]

I'm not sure if we should add this, but open to be convinced otherwise.

There is already a way to know if a connection is a mediator connection. You can check if there is a mediaton record assoaciated with the connection id.

Adding the tag means it can get out of sync. What if we remove the mediation record? Then the tag should also be removed. What if the connection is also used for other purposes than just mediation? To me this feels like something that should be handled on the application level.

[genaris]

I agree with @TimoGlastra in that this seems a bit application-specific. We are also using in our app this concept of 'connection type' but use other roles than the 'Mediator' used in this case, and I guess others might use it differently as well.

That being said, I think it will be nice to improve in AFJ the access to setting and querying tags from outside the framework. Currently, in order to set a tag to a record we have to set the new tag in the record, then resolve the associated service or repository and call update(), etc. And also a similar procedure to find by tag (using findByQuery or findSingleByQuery).

I won't say that this is impossible or very hard currently, but tags are a good feature to classify and efficiently search for records and it would be nice to leverage its usage in a general-purpose manner.

[TimoGlastra]

Agreed @genaris, we should probably expose the findByQuery and update methods on the public api of all modules.

[KolbyRKunz]

What we are thinking we are wanting to do is go and change the connectionType tag to be of [ConnectionType | string] so that the end application developer can add their own custom tags while supporting our own defined tags. The other goal we have is to create methods within the module that will allow for the getting and setting of these tags from the application level along with methods to generically handle the accessing of tags at the application level. The Idea behind making a specific method for updating the connectionType is to make is obvious for the developer using AFJ to see that it is a value for them to use instead of having every developer have to learn what every tag on a connection record is, while at the same time still allowing those who know what is going on internally in AFJ to have access to the other tags on the record. In the future if more tags are added then we can create more specific methods for those tags at that time or for other existing tags. @TimoGlastra @genaris

[genaris]

Well, if this solves the problem for you I think it's fine to add this convenient connectionType tag, as long as it's not limited to a single or couple of possible values (as today is for ConnectionType.Mediator) so it allows each developer to define their own types besides the default ones. It would be certainly useful to have something to easily categorize connections and efficiently search for a bunch of them.

@TimoGlastra suggested today to use string array for this tag, which might be even more useful as we could be more specific when searching for connections (e.g. a connection could be a Mediator and also an Issuer or a Verifier or whatever the application uses). I have not used arrays for tags so I don't know about the complexity of using it, but I think it's worth to take a look at this possibility.

To make this really usable besides this particular case, I think it will be important to add a module-level method in Connections that allows to search for connection type. For instance something like findAllByConnectionType() so we can search for particular types without the need of retrieving all records and then filtering in JS (this could become significant when the wallet has lots of connections and records).

[KolbyRKunz]

@TimoGlastra @genaris Made some updates to this, the main thing being that connectionType is now an array accepting strings or values from the connectionType enum. I also added some methods to the connectionModule that allows you to give a connectionId and view the Tags on that record along with methods to modify the connectionType array from the module. Currently I figured the only Tag we want to expose to direct modification is the connectionType tag, the other two methods added are purely for viewing the tags on the record by either getting all of them or just one specified. Let me know if there is any feedback you have.

[genaris]

Looks nice and useful. I left a few comments regarding getTagFromRecord and getAllTagsFromRecord that IMHO are not necessary.

If possible, I would like to add the request of adding a method called findAllByConnectionType that allow us to query all connections that match the specified type/s. This would be useful for classifying the connections and querying in an efficient way. For instance, suppose that we want to retrieve only the mediators or the connections that the user tagged as 'friends': this query will do a direct request on the wallet backend instead of retrieving all the records and then filtering in the frontend.

[JamesKEbert]

Hey @TimoGlastra, the CI appears to be failing on this PR, in particular though the Postgres wallet is failing to get setup, so it appears to be unrelated to changes made in this PR. Is this related to the same CI issues w/the BBS signatures branch(es)?

[genaris]

Hey @TimoGlastra, the CI appears to be failing on this PR, in particular though the Postgres wallet is failing to get setup, so it appears to be unrelated to changes made in this PR. Is this related to the same CI issues w/the BBS signatures branch(es)?

Seems to be something different, as this is on the main branch and the issue with BBS signatures was in 0.3.0-pre branch. I noticed that yesterday there were other weird issues in CI with other PR as well.

Looking at the logs I see that it is getting a new version of rust:

info: latest update on 2022-09-22, rust version 1.64.0 (a55dd71d5 2022-09-19)

Probably it has to do with that? There is a related issue here which apparently has to do with some new dependencies. Dependency hell not only present in JavaScript 😄

   Compiling socket2 v0.3.11
error[E0512]: cannot transmute between types of different sizes, or dependently-sized types
   --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/socket2-0.3.11/src/sockaddr.rs:156:9
    |
156 |         mem::transmute::<SocketAddrV4, sockaddr_in>(v4);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: source type: `SocketAddrV4` (48 bits)
    = note: target type: `sockaddr_in` (128 bits)

[sairanjit]

@JamesKEbert @genaris @TimoGlastra this is the issue with the rust dependencies in 1.64.0 but it works fine with 1.63.0 Can I create a PR to by default use rust version 1.63.0

I ran the tests here https://github.com/sairanjit/aries-framework-javascript/actions/runs/3115296783/jobs/5052043298 it is working fine changes are here #1036

[genaris]

@KolbyRKunz could you update the branch so CI is relaunched? It should work now with the fix done by @sairanjit.

Also I have my last suggestion: to add findAllByConnectionId at module level (ConnectionsModule.ts) in addition to the method in ConnectionService you've already added (i.e. the module-level method internally calls the service-level method, like it happens with findById).