pulumi / pulumi/pulumi-java

Refactor internals for easier testing and less complexity

Open
#626 2 comments 0 reactions 1 assignee View on GitHub

@pawelprazak is already working on this.

Since May 24, 2022.

area/languages kind/enhancement language/java
Dominant language
Java
Stars
85
Forks
26
Avg merge
11h 49m
Merged PRs (30d)
22

Description

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

This issue by nature probably will not have a complete description since we might learn new things while implementing and I might have forgotten some more things that should be added.

I'll do my best to keep this list up to date as we learn more.

The goals:

  • allow for adding features and writing tests easily
  • reduce internal complexity for less costly maintenance
  • allow for maximum reuse and coherence between CLI, Automation and Tests

Non-goals:

  • we do not want to diverge too much from TS and Go and other implementations
  • we do not want any big changes to the public API

Problems and solutions are described below.

Focused mainly on Deployment:

  • DeploymentImpl reads environment variables directly instead being configured using the constructor, this creates problems with testing and violates single responsibility principle - proposed solution is to read the environment variables in PulumiInternal and pass an "options object" (RuntimeContext?) with the result along with other dependencies to the constructor (this will also help with implementing the Automation API)
  • DeploymentImpl creates its own dependencies internally and that does not allow for easy testing - proposed solution is to use manual dependency injection and pass all dependencies as parameters to the constructor, this allows for cleaner PulumiInternal and for a cleaner testing framework with less mocking
  • DeploymentImpl holds its dependencies as nested classes, that makes the file big and hard to work with, encourages tight coupling between separate parts, does not allow for easy use in testing framework - proposed solution is to move all nested internal classes to package com.pulumi.runtime.internal and also extract interfaces for all of those classes (e.g Call, Invoke, etc.), as an additional benefit this will allow for easier understanding of the underlying interaction with the engine
  • Deployment is being accessed through a singleton in many places, deep in the code structure, this makes testing harder requiring deployment instance in places where a unit test would be more appropriate - proposed solution is to reduce the amount of calls to the singleton by passing the instance from the edges of the code
  • Deployment as a class name suggests that it is a thing, when in reality it is a state associated with an action, this is misleading - proposed solution is a rename to DeploymentContext and a move to package com.pulumi.context.internal
  • Stack can probably be passed as a DeploymentImpl constructor parameter instead of through a setter, resulting in simplicity and immutability
  • exposure of Deployment to the internals and providers that result in tight coupling could be reduced or eliminated by using "view" interfaces with only a slice of the api targeting a specific audience (e.g. only Call or only registering)

Focused on Resource and its subclasses:

  • ComponentResource might benefit from making registerOutputs a template method to make it obvious to the user that they are required to implement it (instead of relying on the user to remember to call it)
  • ResourceReference as a Resource super-class instead of a sub-class would remove accidental complexity
  • Stack, StackReference and WorkspaceStack should have coherent APIs
  • Stack should be final and not instantiatable directly by the user
  • consider reuse of Stack class behind a WorkspaceStack interface for the Automation API

Other:

  • Pulumi should get a companion interface with the public test framework API called PulumiTest that would contain mostly what DeploymentMockBuilder and DeploymentMock do right now; also backed by the same PulumiInternal, probably extended with PulumiTestInternal for any test specific implementations
  • Pulumi should get a companion interface with the public Automation API called PulumiAuto or PulumiAutomation also backed by the same PulumiInternal
Affected area/feature

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.