Remove Optional Injection intended for Mocking

[CMCDragonkai]

Specification

class X {}

class Y {
  constructor(x?: X) {
  }
}

// There should be a difference in behaviour between the two

const yEncapsulated = new Y();
// * Lifecycle of X is managed on the inside
// * Events is encapsulated

const yInjected = new Y(new X());
// * Lifecycle of X is managed on the outside
// * Events is not encapsulated

When separating the functionality between encapsulated dependencies and injected dependencies, we have to basically have a boolean that branches out the functionality.

However this results in alot of boilerplate. Requiring a separate xIsInjected boolean for every dependency that may be encapsulated or injected.

Only a few dependencies make use of this distinction during runtime. Most of the time, this is actually only used for testing & mocking.

Consider for example in the 3 situations of EFS, QUIC. In EFS, the db can be encapsulated or injected. In QUIC, the QUICSocket can be encapsulated or injected. Both cases are legitimate usecases. We should definitely have a boolean that indicates the distinction.

Consider the case of PolykeyAgent. Almost all of its dependencies should just be fully encapsulated. The only reason to enable the ability to pass in DB,... etc is allow mocking or unit testing. But the amount of boilerplate to support all of the dependencies would be HUGE!

The only way to make such a thing manageable would be to introduce parameter decorators on the constructor that automatically produces properties.

However I don't want to do that right now, it might introduce too much complexity. Instead let's do something different.

1. Where we have legitimate reasons for encapsulation/injection, we continue doing it with a manually specificed xIsInjected or equivalent boolean.
2. Where it's only there for testing, we can remove it..., and instead rely on module mocking in jest to make use of it. See: https://jestjs.io/docs/mock-functions#mocking-modules

This doesn't have to be a wholesale change immediately... we can just keep an issue in PK and slowly apply this across the ecosystem. In some cases, optional injection can still be kept since there's barely any lifecycle related activities, or the relevant objects in question are not evented objects.

Additional context

• Events Refactoring (integrating js-events) js-quic#53 (comment)

Tasks

1. Identify places where optional dependencies are only intended for testing like in PolykeyAgent and remove them.
2. Replace tests that inject into them with just module mocking https://jestjs.io/docs/mock-functions#mocking-modules
3. In places where injection is actually a useful thing to do, don't touch them but ensure their lifecycles are managed outside if so.

[CMCDragonkai]

@tegefaulkes was this done?

[tegefaulkes]

I think so, I removed the optional DI from PolykeyAgent. If none of the other domains need this treatment then yeah, it's done.

[CMCDragonkai]

Were there any module mocking that was needed?

And what about PolykeyClient?

[tegefaulkes]

May have been some changes to mocking, I don't recall.

The Polykeyclient doesn't do any optional DI for testing.

[CMCDragonkai]

Ok then it is done for now. Will focus on rest of testnet 6 issues.