finos / finos/architecture-as-code

Refactor Hub Command Client for maintainability

Open
#2,516 0 comments 0 reactions 1 assignee Claimed by @jimthompson5802 View on GitHub
Dominant language
TypeScript
Stars
399
Forks
138
Avg merge
2d 14h
Merged PRs (30d)
37

Description

per comment: https://github.com/finos/architecture-as-code/pull/2472#discussion_r3299524553

refactor `cli/src/command-helpers/hub-commands.ts` for maintainability.

Will address this issue after PR #2472 is merged.

One possible apporach for refactoring

# Hub Commands Refactor Plan: `cli/src/command-helpers/hub-commands.ts`

Developed with the help of OpenAI Codex coding assistant with this prompt:

```
review hub-commands.ts. Is it possible to break this into smaller modules with a common theme.
Are there any functions that would be shared among the smaller modules that can be placed in
separate "common" module. Do not change anyting. Just produce a report I can review.
```

## Recommended Module Split

Split by resource family, with one shared common module:

```text
cli/src/command-helpers/hub/
common.ts
versioned-documents.ts
namespaces.ts
domains.ts
controls.ts
index.ts
```

### `common.ts`

- `HubCommandError`
- `resolveCalmHubOptions`
- `handleOptionsLoadError`
- `handleHubError`
- Common file helpers
- Common ID parsing helpers
- Common output helpers like `printPushResult`, `printIdCreateResult`

### `versioned-documents.ts`

- Architecture, pattern, and standard commands:
- `runPushArchitecture`, `runPullArchitecture`, `runListArchitectures`
- `runPushPattern`, `runPullPattern`, `runListPatterns`
- `runPushStandard`, `runPullStandard`, `runListStandards`
- Shared versioned-document helpers:
- Generic metadata resolver
- Generic "read file and validate JSON"
- Generic pull-to-stdout-or-file

### `namespaces.ts`

- `runCreateNamespace`
- `runListNamespaces`

### `domains.ts`

- `runCreateDomain`
- `runListDomains`

### `controls.ts`

- `runCreateControlRequirement`
- `runListControlRequirements`
- `runPushControlRequirement`
- `runPullControlRequirement`
- `runPushControlConfiguration`
- `runPullControlConfiguration`
- `runCreateControlConfiguration`
- `runListControlConfigurations`

### `index.ts`

- Re-export the same public functions so `cli.ts` can keep importing from one place.

## Common Helpers Worth Extracting

- `readAndValidateJsonFile(file, commandLabel, format)`
Used repeatedly by push/create commands before sending file contents.

- `parseIntegerOption(value, optionName, commandLabel, format)`
Repeated for `--id`, `--control-id`, and `--config-id`.

- `writeJsonOrPrint(result, output?)`
Repeated by architecture, pattern, standard, control requirement, and control configuration pulls.

- `createHubClient(calmHubOptions, format)`
Wraps `handleOptionsLoadError` and `new CalmHubClient(...)`.

- `resolveVersionedResourceMetadata(...)`
Could replace the separate architecture, pattern, and standard metadata resolvers by accepting:
- List function
- Resource label
- Command label
- Namespace
- ID/name/description

- `runVersionedDocumentPush(...)`
Architecture, pattern, and standard pushes are nearly the same: validate create-required metadata, read JSON, then either create new or push version.

## Best Refactoring Order

1. Add `hub/common.ts`, moving only error/options/output/file/id helpers first.
2. Move namespace/domain commands, since they are smaller and lower risk.
3. Move architecture/pattern/standard commands into `versioned-documents.ts`.
4. Move controls/configurations last, because they have the most special-case behavior.
5. Add `hub/index.ts` and either update `cli.ts` imports once, or keep a temporary compatibility re-export from the old `hub-commands.ts`.

## Caution Points

- `hub-commands.spec.ts` imports several helpers directly, not just `run*` commands. A split will require either test import updates or a compatibility barrel.
- `handleHubError` currently calls `process.exit(1)`, so helpers that call it are effectively `never`; keep that behavior unchanged during the split.
- Avoid over-generalizing controls too early. Versioned documents share a clean pattern; controls have enough domain-specific behavior that they should mostly stay explicit.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.