Client Domain Structure of Callers and Handlers should match Agent Callers and Handlers

[addievo]

Description

The client domain is out of spec with the updated node domain. The goal of this PR is to match spec of client domain with nodes domain.

Issues Fixed

• Fixes Update client domain structure #563
• Fixes Convert ClientService to StartStop #565
• Fixes Use default exports for handler classes #520

Tasks

• 1. All the handlers were updated to use default exports.
• 2. All the handlers were updated to use arrow functions.
• 3. The ServerManifest was moved into the index.ts.
• 4. The caller definitions were moved into client/callers.
• 5. The caller definitions were split into separate files.
• 6. clientManifest was moved into client/callers/index.ts.
• 7. errors.ts, types.ts and utils.ts are in client directory now.
• 8. Convert ClientService to startStop.
• 9. Update everything in tests/client

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[ghost]

👇 Click on the image for a new way to code review

Legend

[CMCDragonkai]

Yea make sure that src/client/callers and src/client/handlers look exactly the same as src/nodes/agent.

Default exports, and utility function structure, and update all imports elsewhere, and have a look at tests too.

Make sure to rebase on staging when ready for review too.

[CMCDragonkai]

Name of the PR should be more descriptive. I'm changing it.

[CMCDragonkai]

I think #565 should also be fixed here.

[addievo]

@tegefaulkes go over ClientService, there might be a little things wrong about it, I couldn't quite figure out how to implement StartStop that well

[tegefaulkes]

Two small things stand out, otherwise seems good.

[CMCDragonkai]

I'm taking over this. Will note all things missing.

1. src/client/index.ts isn't aligned.
2. src/client/middleware.ts and src/client/timeoutMiddleware.ts and src/client/authenticationMiddleware.ts is really awkward. This might be better put together.
3. Code isn't linted.
4. Probably not tested.
5. Imports isn't aligned.
6. Spacing isn't aligned.
7. Indeed the import type XHandler should have been import type X.
8. Dependencies should be type imports for src/client/handlers/index.ts

@addievo don't touch anything.

[CMCDragonkai]

Client service I think is a bit broken. Will need to fix it.

[CMCDragonkai]

Rebased on top of staging.

There are test failures here... could it be due to import changes?

[CMCDragonkai]

Agent handlers were not aligned properly... just noticed this. File naming wasn't done correctly.

[CMCDragonkai]

Ok so now we are just in the tedious process of updating all the tests/client imports and ClientService construction. Furthermore, alot of the tests is repeating the setup alot, they were originally separate files originally but @tegefaulkes gathered them together to reduce memory usage.

[CMCDragonkai]

Should be finished in a next hr or so, just need confirmation on why that password behaviour is happening. @tegefaulkes

[CMCDragonkai]

All tests in tests/client is passing now. Some things of note:

1. Import order and import deduplication has not been maintained, this resulted in lots of messy imports that was very confusing. This needs discipline here.
2. The keysPublicKey command does not take a password message.
3. The notificationsSend handler had the wrong type.
4. All the cases where we are repeating test setup should be moved into describe.
5. There's a lack of care taken when writing the order of declarations, the order should be natural following the order of usage.
6. I think overall there's too much reliance on the IDE happening here, here's a simple trick, go get the ag command and always run it against your directory to see if things are being mis-used, and always check your imports and exports before you finish. This can save alot of time at the end.
   gs
   Most importantly! The vault tests have todos for some of the most important commands!!! These are the commands that we are actually demoing, and these require actual tests.

✎ todo clones a vault
✎ todo pulls from a vault
✎ todo scans a vault

Why are we testing small things, and not the big things? This needs to be done as we are deploying and reviewing the UX.

[CMCDragonkai]

BTW @amydevs when sanity checking final build, what I do is npm run build then I do a node and I do require('./dist/index.js').