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

Epic VDR phase 3 branch #1245

Closed
wants to merge 32 commits into from
Closed

Epic VDR phase 3 branch #1245

wants to merge 32 commits into from

Conversation

shotexa
Copy link
Contributor

@shotexa shotexa commented Jul 2, 2024

Description:

Epic branch for https://input-output.atlassian.net/browse/ATL-6543

This Epic PR contains commits from other feature PRs that have been reviewed separately

Closed in favor of #1385

Alternatives Considered (optional):

Link to existing ADR (Architecture Decision Record), if any. If relevant, describe other approaches explored and the selected approach. Documenting why the methods were not selected will create a knowledge base for future reference, helping prevent others from revisiting less optimal ideas.

Checklist:

  • My PR follows the contribution guidelines of this project
  • My PR is free of third-party dependencies that don't comply with the Allowlist
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked the PR title to follow the conventional commit specification

@shotexa shotexa marked this pull request as draft July 2, 2024 14:32
Copy link
Contributor

github-actions bot commented Jul 2, 2024

Integration Test Results

20 files  ±0  20 suites  ±0   2s ⏱️ ±0s
45 tests ±0  45 ✅ ±0  0 💤 ±0  0 ❌ ±0 
71 runs  ±0  71 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b59ce50. ± Comparison against base commit 693dcc4.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jul 2, 2024

Unit Test Results

102 files  +1  102 suites  +1   16m 13s ⏱️ - 4m 27s
872 tests +3  864 ✅ +3  8 💤 ±0  0 ❌ ±0 
879 runs  +3  871 ✅ +3  8 💤 ±0  0 ❌ ±0 

Results for commit b59ce50. ± Comparison against base commit 693dcc4.

♻️ This comment has been updated with latest results.

Shota Jolbordi and others added 15 commits July 17, 2024 04:21
Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: Hyperledger Bot <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Hyperledger Bot <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: FabioPinheiro <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: Yurii Shynbuiev - IOHK <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: Bassam Riman <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: Pat Losoponkul <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: Pat Losoponkul <[email protected]>
Signed-off-by: Hyperledger Bot <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Hyperledger Bot <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: Yurii Shynbuiev - IOHK <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
…ci] (#1257)

Signed-off-by: Yurii Shynbuiev <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: Pat Losoponkul <[email protected]>
Signed-off-by: Hyperledger Bot <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Hyperledger Bot <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: Allain Magyar <[email protected]>
Signed-off-by: Hyperledger Bot <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Hyperledger Bot <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
# [1.38.0](cloud-agent-v1.37.0...cloud-agent-v1.38.0) (2024-07-15)

### Bug Fixes

* Move InMemory classes to the test moduels ([#1240](#1240)) ([823057a](823057a))
* move mocks into the test modules ([#1236](#1236)) ([df83026](df83026))
* use Put and Get for DID in doobie statement ([#1250](#1250)) ([fc1cf51](fc1cf51))
* Wallet Management Error Handling ([#1248](#1248)) ([cfd5101](cfd5101))

### Features

* upgrade docusaurus and semantic-release packages ([de53f1d](de53f1d))

Signed-off-by: Allain Magyar <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: Pat Losoponkul <[email protected]>
Signed-off-by: patlo-iog <[email protected]>
Signed-off-by: Hyperledger Bot <[email protected]>
Co-authored-by: Yurii Shynbuiev - IOHK <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Hyperledger Bot <[email protected]>
Signed-off-by: Shota Jolbordi <[email protected]>
@coveralls
Copy link

coveralls commented Sep 12, 2024

Coverage Status

coverage: 48.548% (+0.02%) from 48.532%
when pulling b59ce50 on ATL-6543-epic-vdr-phase-3
into 693dcc4 on main.

@shotexa shotexa changed the title [WIP] Epic VDR phase 3 branch Epic VDR phase 3 branch Sep 19, 2024
@shotexa shotexa marked this pull request as ready for review September 19, 2024 19:22
@shotexa shotexa requested a review from a team as a code owner September 19, 2024 19:22
Comment on lines +78 to +87
val res = for {
cd <- service.getByGUID(guid, ResourceResolutionMethod.did)
response <- ZIO
.fromEither(CredentialDefinitionDidUrlResponse.asPrismEnvelopeResponse(cd, baseUrlServiceName))
.mapError(couldNotParseCredDefResponse)

} yield response

res
}
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to use a for-comprehension without val res in the beginning and res in the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its a stylistic choise, I like this because its easier to debug later, and see the response type by hovering over.

.withBaseUri(rc.request.uri)
)
): IO[ErrorResponse, PrismEnvelopeResponse] = {
val res = for {
Copy link
Member

Choose a reason for hiding this comment

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

same here. What is the purpose of this code style?

.mapError(_ => MissingRequiredParams(uri))
(resourceService, resourcePath) = serviceAndPath
didStr = parsed.removeQueryString().toString
didDocument <- didResolver.resolve(didStr).flatMap {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this way of error detail complies with the current error-handling strategy.
Let's commit to it as it is, but discuss it with @bvoiturier at the AoH meeting.

class DidUrlResolver(httpUrlResolver: HttpUrlResolver, didResolver: DidResolver) extends UriResolver {
import DidUrlResolver.*

def resolve(uri: String): IO[GenericUriResolverError, String] = {
Copy link
Member

Choose a reason for hiding this comment

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

I would also try to refactor the long function and split it into the stages/phases:

  1. parse Uri => get DID
  2. resolve DID
  3. fetch Services
  4. compose the resource Url
  5. get the resource by Url
  6. check resource integrity (if needed)

ZIO.fail(UnexpectedUpstreamResponseReceived(status.code))
}
} yield body
program.provideSomeLayer(zio.Scope.default)
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of the Scope.default here?

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'm not sure It is a refactored code, but if I recall, ZIO HTTP client needs this layer to function

import org.hyperledger.identus.pollux.vc.jwt.{CredentialPayload, DidResolver, JWT, JWTVerification, JwtCredential}
import org.hyperledger.identus.pollux.vc.jwt.CredentialPayload.Implicits
import org.hyperledger.identus.shared.http.UriResolver
// import org.hyperledger.identus.pollux.vc.jwt.CredentialPayload.Implicits //TODO: might not be necessary
Copy link
Member

Choose a reason for hiding this comment

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

you can always run optimize imports from the Intellij Idea

@shotexa
Copy link
Contributor Author

shotexa commented Sep 27, 2024

Closing this one due to DCO issues, see #1385

@shotexa shotexa closed this Sep 27, 2024
@shotexa shotexa deleted the ATL-6543-epic-vdr-phase-3 branch September 27, 2024 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants