finos / finos/architecture-as-code
Refactor Hub Command Client for maintainability
- 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
Assessment
This issue has not been assessed yet.