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

First pass at DAP resolution #3

Merged
merged 3 commits into from
Jun 25, 2024
Merged

First pass at DAP resolution #3

merged 3 commits into from
Jun 25, 2024

Conversation

mistermoe
Copy link
Member

@mistermoe mistermoe commented Jun 25, 2024

First pass at implementing DAP Resolution. There will almost certainly be some refactoring in subsequent PRs, but wanted to get something out that works.

Warning

  • low test coverage
  • test isn't using mocks
  • not entirely sold on class names yet
  • it feels like the word Resolver gets used 18,327 times. You'll see in the Usage section below

Usage

import 'package:dap/dap.dart';
import 'package:web5/web5.dart';

void main() async {
  final dap = Dap.parse('@moegrammer/didpay.me');
  print(dap.handle); // moegrammer
  print(dap.domain); // didpay.me
  print(dap.registryDid); // did:web:didpay.me
  print(dap); // @moegrammer/didpay.me

  // TODO: use mocks
  final resolver = DapResolver(DidResolver(
    methodResolvers: [DidWebResolver()],
  ));

  final result = await resolver.resolve(dap);
  print(result.moneyAddresses);
}

Comment on lines 27 to 32
final registryServices = registryDidResolution.didDocument!.service ?? [];
final dapRegistryService = registryServices.firstWhere(
(service) => service.type == 'DAPRegistry',
orElse: () => throw DapResolutionException(
'Registry DID does not have a DAPRegistry service'),
);
Copy link
Member

Choose a reason for hiding this comment

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

nit:
Since you're handing the orElse in the 2nd case, you might want to avoid force unwrapping the didDocument! in the first. Maybe something like:

final dapRegistryService = registryDidResolution.didDocument?.service.firstWhereOrNull((s) => s.type == 'DAPRegistry');
if (dapRegistryService == null) {
  throw DapResolutionException(
          'Registry DID does not have a DAPRegistry service');
}

(this code is probably wrong since I just wrote it in Github 😂 )

Copy link
Member

Choose a reason for hiding this comment

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

There's a few other places where force unwarpping is happening here too. Either way is cool, but I tend to lean towards never force unwrapping expect in specific cases in widget layout trees

Comment on lines 34 to 37
if (dapRegistryService.serviceEndpoint.isEmpty) {
throw DapResolutionException(
'DAPRegistry service does not have a service endpoint');
}
Copy link
Member

Choose a reason for hiding this comment

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

nit:
You could grab the endpoint here so you don't need to use first below if you want.

final serviceEndpoint = dapRegistryService.serviceEndpoint.firstOrNull;
if (serviceEndpoint == null) {
  throw DapResolutionException('DAPRegistry service does not have a service endpoint');
}

// then below
registryEndpoint = Uri.parse(serviceEndpoint)

@mistermoe
Copy link
Member Author

thanks for the comments @wesbillman ! I agree that force unwrapping is a footgun. created an issue to remove all force unwrapping in a subsequent PR: #4. Also created an issue in web5-dart to make didResolutionResult.didDocument not nullable because ... it shouldn't be! TBD54566975/web5-dart#93

@ethan-tbd
Copy link
Contributor

thanks for the comments @wesbillman ! I agree that force unwrapping is a footgun. created an issue to remove all force unwrapping in a subsequent PR: #4. Also created an issue in web5-dart to make didResolutionResult.didDocument not nullable because ... it shouldn't be! TBD54566975/web5-dart#93

just saw this comment! latest commit actually closes TBD54566975/web5-dart#93 :))

@ethan-tbd ethan-tbd merged commit 87ef17d into main Jun 25, 2024
1 check passed
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.

3 participants