Skip to content
This repository has been archived by the owner on Dec 12, 2024. It is now read-only.

Add exchange state machine #168

Merged
merged 6 commits into from
Feb 13, 2024
Merged

Add exchange state machine #168

merged 6 commits into from
Feb 13, 2024

Conversation

diehuxx
Copy link

@diehuxx diehuxx commented Feb 9, 2024

This state machine class Exchange will be used to great effect in a follow-up PR in the http-server package.

Copy link

changeset-bot bot commented Feb 9, 2024

🦋 Changeset detected

Latest commit: e755787

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@tbdex/protocol Patch
@tbdex/http-client Patch
@tbdex/http-server Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Feb 9, 2024

TBDocs Report

🛑 Errors: 0
⚠️ Warnings: 12

@tbdex/protocol

  • Project entry file: packages/protocol/src/main.ts

@tbdex/http-client

  • Project entry file: packages/http-client/src/main.ts
📄 File: packages/http-client/src/errors/request-error.ts
⚠️ extractor:ae-missing-release-tag: "RequestErrorParams" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) #L1
⚠️ extractor:ae-undocumented: Missing documentation for "RequestErrorParams". #L1
⚠️ extractor:ae-undocumented: Missing documentation for "recipientDid". #L13
⚠️ extractor:ae-undocumented: Missing documentation for "url". #L14
📄 File: packages/http-client/src/errors/request-token-error.ts
⚠️ extractor:ae-missing-release-tag: "RequestTokenErrorParams" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) #L3
⚠️ extractor:ae-undocumented: Missing documentation for "RequestTokenErrorParams". #L3
📄 File: packages/http-client/src/errors/response-error.ts
⚠️ extractor:ae-missing-release-tag: "ResponseErrorParams" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) #L3
⚠️ extractor:ae-undocumented: Missing documentation for "ResponseErrorParams". #L3
⚠️ extractor:ae-undocumented: Missing documentation for "statusCode". #L15
⚠️ extractor:ae-undocumented: Missing documentation for "details". #L16
⚠️ extractor:ae-undocumented: Missing documentation for "recipientDid". #L17
⚠️ extractor:ae-undocumented: Missing documentation for "url". #L18

@tbdex/http-server

  • Project entry file: packages/http-server/src/main.ts

TBDocs Report Updated at 2024-02-13T17:54:29Z e755787

@diehuxx diehuxx force-pushed the exchange-state-machine branch from d49e4fc to 8a5ee23 Compare February 9, 2024 00:36
@diehuxx diehuxx force-pushed the exchange-state-machine branch from 8a5ee23 to ced180a Compare February 9, 2024 00:40
Copy link

codecov bot commented Feb 9, 2024

Codecov Report

Merging #168 (e755787) into main (552675c) will increase coverage by 3.58%.
Report is 3 commits behind head on main.
The diff coverage is 96.82%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #168      +/-   ##
==========================================
+ Coverage   85.55%   89.13%   +3.58%     
==========================================
  Files          35       36       +1     
  Lines        2803     2871      +68     
  Branches      234      268      +34     
==========================================
+ Hits         2398     2559     +161     
+ Misses        405      312      -93     
Components Coverage Δ
protocol 93.51% <96.82%> (+3.49%) ⬆️
http-client 93.62% <ø> (+6.83%) ⬆️
http-server 71.15% <ø> (-0.41%) ⬇️

Copy link
Member

@mistermoe mistermoe left a comment

Choose a reason for hiding this comment

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

amajing

it('adds an Rfq', async () => {
const aliceDid = await DevTools.createDid()
const pfiDid = await DevTools.createDid()
const rfq = Rfq.create({
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we have devtool methods to create messages

Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to have a top level beforeEach() where the messages are defined for each nested describe() tests? then you don't have to repeat them for each nested describe()

Copy link
Author

Choose a reason for hiding this comment

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

The devtool methods are useful for creating a message with any random metadata, but it's more concise to use the .create() methods in this case so we can specify to, from, and exchangeId.

I updated beforeEach() according to your suggestion.

exchangeId : rfq.metadata.exchangeId,
},
data: {
reason: 'I dont like u anymore'
Copy link
Contributor

Choose a reason for hiding this comment

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

😢

}
}
})

Copy link
Contributor

Choose a reason for hiding this comment

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

i think this test is checking for the messages we can't add after rfq - could we have similar "negative" tests for quote, order, orderstatus, and close?

i.e.

  • quote: cannot add orderstatus after quote
  • order: cannot add rfq or quote after order
  • orderstatus: cannot add rfq, quote, order after orderstatus

Copy link
Author

Choose a reason for hiding this comment

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

Good call. Done.

@@ -0,0 +1,5 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

do they tho? i feel like cheetahs aren't good at sharing.

Copy link
Author

Choose a reason for hiding this comment

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

green cheetahs are envious cheetahs

@diehuxx diehuxx force-pushed the exchange-state-machine branch from b42642a to e755787 Compare February 13, 2024 17:52
@diehuxx diehuxx merged commit 589edc3 into main Feb 13, 2024
16 of 17 checks passed
@diehuxx diehuxx deleted the exchange-state-machine branch February 13, 2024 17:58
phoebe-lew added a commit that referenced this pull request Feb 20, 2024
* main:
  Stricten, test, and bugfix http-server (#170)
  Version Packages (#167)
  Update web5/dids, web5/credentials, web5/crypto, web5/common to latest  (#177)
  Add exchange state machine (#168)
  Stricten http-client (#169)
  Make `pfiDid` required property in `TbdexHttpServer` (#166)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants