Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

O1 update #44

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
211 changes: 120 additions & 91 deletions account/README.adoc

Large diffs are not rendered by default.

58 changes: 58 additions & 0 deletions account/prompts/improve-code.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
= Developer Prompt: Improving Classes for AMS

:author: Peter Lawrey
:revdate: 2024-12-16
:revnumber: 1.0
:doctype: book
:toc: left
:toclevels: 3

You are given a set of requirements and an existing implementation of an Account Management Service (AMS). The AMS processes account-related commands (events) such as account creation, fund transfers, and checkpoint requests, and produces corresponding success/failure events. The code currently meets the basic functional requirements but can be improved for clarity, maintainability, and robustness.

Your task is to improve selected classes in the codebase. Consider the following options and guidelines:

== Guidelines

1. **Adhere to the Provided Requirements**:
The code must continue to fulfill the requirements specified in the `Account Management Service Requirements` document. Any changes should not break the contract defined there.

2. **Validation and Error Handling**:
Assume that the framework validates that DTOs (Data Transfer Objects) before use. Add a comment showing where validation would otherwise occur. If a command is missing critical fields or contains invalid values, handle it gracefully by producing failure events rather than exceptions visible to callers.

3. **Time Management**:
All events should use `SystemTimeProvider.CLOCK.currentTimeNanos()` to set their `sendingTime` fields, ensuring nanosecond-precision wall clock timestamps.

4. **Logging and Comments**:
Add meaningful comments where appropriate to explain the rationale behind certain decisions, especially if the code deviates from typical patterns. Consider using `Jvm.debug()`, `Jvm.warn()`, and `Jvm.error()` for logging. Comments should clarify non-obvious logic, error handling decisions, or performance trade-offs. Do not add comments for trivial logic.

5. **Fluent Interfaces and Deep Copies**:
Preserve the fluent interface style for DTO setters to allow method chaining. When storing new accounts, ensure that `CreateAccount` objects are deep copied before saving them to the internal map, as per the requirements.

6. **Checkpoints and State Serialization**:
During checkpoint processing, ensure that all currently known accounts are emitted as `onCreateAccount` events. Consider how to handle any edge cases (e.g., empty account lists).

7. **Readability and Maintainability**:
Consider extracting common logic (e.g., target checks, currency checks, funds checks) into separate helper methods to reduce code repetition. Make sure your class-level and method-level documentation provides a clear picture of what the code does, why, and how it aligns with the requirements.

== Options to Consider

* Add Javadoc to all classes and their public methods, describing the class’s role, its main responsibilities, and linking it back to the requirements.
* Introduce private helper methods to streamline complex validation or repetitive tasks.
* Use descriptive variable and method names to enhance clarity.
* Check that all failure events include a meaningful `reason` field that matches the requirements.
* Consider adding `@Override` annotations, if missing, to clarify implemented methods from interfaces.
* Add informative comments that explain why certain validations or steps are necessary, rather than just stating what the code does.
* Ensure that the codebase is consistent in its style and adheres to the project’s coding standards.
* Consider how to handle edge cases and exceptional conditions, ensuring that the code behaves predictably and correctly in all scenarios.

== Deliverables

Improve the existing codebase by addressing the guidelines and options provided. Submit the updated classes with the changes you have made, along with a brief summary of the modifications you implemented and why you chose to make them.

== Objective

By following the above guidelines and considering the options, improve the existing codebase to be more robust, understandable, and aligned with the specified requirements. The resulting classes should present a clean, well-documented, and maintainable code structure that clearly communicates their purpose and logic.

== Code To Improve

Find the code to improve below:
149 changes: 149 additions & 0 deletions account/prompts/improve-test-data.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
= Developer Prompt: Improving Test Cases for AMS

Your task is to enhance an existing test configuration that utilizes YAML files for initializing system state (`_setup.yaml`), specifying input commands (`in.yaml`), and verifying expected outcomes (`out.yaml`). The goal is to produce maintainable, clear, and requirements-aligned test cases.

== Overview

The testing approach involves the following files:

* `_setup.yaml` - Initializes the system state before the test scenario begins (e.g., creating initial accounts).
* `in.yaml` - Defines the input commands (events) that the system under test will process.
* `out.yaml` - Specifies the expected events produced by the system in response to the inputs, along with helpful comments that link the outputs back to the corresponding input events.

Below is an illustrative structure:

.Setup (`_setup.yaml`)
----
---
# Create account for Alice (account #101013) starting with 15 EUR.
# Rationale: This sets up a baseline account state for subsequent operations.
createAccount: {
sender: gw1,
target: vault,
sendingTime: 2023-01-20T10:00:00,
name: alice,
account: 101013,
currency: EUR,
balance: 15
}
...
----

.Input (`in.yaml`)
----
---
# Transfer 10 EUR from Alice (101013) to Bob (101025).
# Scenario: This should succeed if Bob’s account is in EUR and both accounts are valid.
transfer: {
sender: gw2,
target: vault,
sendingTime: 2023-01-20T10:03:00,
from: 101013,
to: 101025,
currency: EUR,
amount: 10,
reference: Dog food
}
...
---
# This operation requests a checkpoint.
# Checkpoints are typically used to dump or save the state of the system at a certain point in time.
# In this case, it will dump all the accounts.
checkPoint: {
sender: gw2,
target: vault,
sendingTime: 2023-01-20T11:00:00,
}
...
----

[source,mermaid]
----
sequenceDiagram
participant SetupFile as _setup.yaml
participant InputFile as in.yaml
participant TestRunner as YamlTester
participant System as System Under Test<br> (AMS)
participant ExpectedOutput as out.yaml

SetupFile->>TestRunner: Load initial state instructions
Note over TestRunner: The Test Runner reads _setup.yaml<br>and applies initial configurations<br> (e.g., create accounts)

TestRunner->>System: Initialize system state<br> from _setup.yaml
Note over System: System now has initial state<br> (e.g., Alice’s account with 15 EUR)

InputFile->>TestRunner: Provide input<br> commands/events
Note over TestRunner: The Test Runner reads in.yaml<br> which includes operations<br> like transfers, checkpoints

TestRunner->>System: Replay input events<br> from in.yaml
Note over System: System processes each command<br>and produces corresponding output events<br> (e.g., onTransfer, createAccountFailed)

System->>TestRunner: Return<br> produced events
Note over TestRunner: The Test Runner captures all events<br>generated by the system<br> in response to input

ExpectedOutput->>TestRunner: Provide expected events (out.yaml)
Note over TestRunner: The Test Runner checks the produced events<br>against the expected output<br> defined in out.yaml

TestRunner->>TestRunner: Compare actual vs expected events
alt All Events Match
TestRunner->>System: Test PASSED
else Some Events Differ
TestRunner->>System: Test FAILED
end
----

== Guidelines

1. **Clarity and Context**:
Add descriptive comments to `_setup.yaml` and `in.yaml` to explain each operation’s intent. In `out.yaml`, reference the input event that caused the output. This makes it easier for other developers to understand the test scenarios at a glance.

2. **Time Management**:
Document that real-time tests should use `SystemTimeProvider.CLOCK.currentTimeNanos()` for `sendingTime`. Though test files may use fixed timestamps, emphasize in comments that production environments rely on `SystemTimeProvider` for consistent, nanosecond-precision timestamps.

3. **Validation Checks**:
Introduce failure scenarios:
* A `createAccount` command with an invalid balance (e.g., negative balance) to produce `createAccountFailed`.
* A `transfer` from a non-existent account or with insufficient funds to produce `transferFailed`.

In `in.yaml`, comment these scenarios and in `out.yaml`, show the expected failure outputs, including a `reason` field that aligns with the system’s requirements.

4. **Reusability and Maintenance**:
If your setup becomes complex, consider YAML anchors, aliases, or splitting large scenarios into multiple files. Add comments linking tricky scenarios to relevant sections of the requirements document, ensuring future maintainers understand the rationale behind each test.

5. **Coverage**:
Include scenarios that cover:
* Multiple successful account creations and transfers.
* At least one invalid `createAccount` scenario.
* At least one invalid `transfer` scenario.
* A `checkPoint` command to verify the sequence of `startCheckpoint`, `onCreateAccount` for each known account, and `endCheckpoint` events.

6. **Naming and Organization**:
Use meaningful and specific operation descriptions. Instead of generic comments, specify the exact accounts, currencies, and reasons. Label scenarios (e.g., "Scenario: Insufficient Funds Transfer") to quickly identify their purpose.

== Sections for Setup and Input Data

- **Setup Section (`_setup.yaml`)**:
Place all initial state operations here. Add comments that justify these initial states and their relevance to the upcoming tests.

----
# Example (in `_setup.yaml`):
# Creating initial accounts to ensure subsequent transfers have valid source and destination accounts.
createAccount: { ... }
...

----

- **Input Section (`in.yaml`)**:
Define the sequence of commands tested. Include both normal and edge cases, clearly tagging scenarios for quick reference.

----
# Example (in `in.yaml`):
# Scenario: Attempt to transfer from a non-existent account to test transferFailed event.
transfer: { ... }
...

----

== Deliverables

Enhance the existing `_setup.yaml` and `in.yaml` files according to the above guidelines. Once updated, provide a brief summary of the changes made and the reasons behind them, focusing on improved clarity, test coverage, and alignment with requirements.
179 changes: 179 additions & 0 deletions account/prompts/requirements.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
= Account Management Service Requirements

:author: Peter Lawrey
:revdate: 2024-12-16
:revnumber: 1.1
:doctype: book
:toc: left
:toclevels: 3

== Introduction

This document specifies the functional requirements for an Account Management Service (AMS) that processes account-related commands and generates corresponding events. The AMS is designed to be driven by incoming commands (events) such as account creation, fund transfers, and checkpoint requests. It produces events to indicate success or failure and to provide state snapshots.

== Terminology

*Account*: A financial store of value characterized by an account number, owner name, currency, balance, and an optional overdraft limit.

*Command*: An inbound request to the system (e.g., create an account, transfer funds, request checkpoint). Commands are modelled as Events in this system.

*Event*: An outbound notification from the system that indicates a command's state change, success, or failure.

*Checkpoint*: A request to serialize or snapshot the current state of all accounts for audit or recovery purposes.

== Functional Requirements

=== Account Creation

==== Inputs

1. A `CreateAccount` command containing:

* `sender`: The origin of the command.
* `target`: The intended system (e.g., `vault`).
* `sendingTime`: Holds when the command was sent as a wall clock timestamp with nanosecond resolution.
* `name`: The account holder's name.
* `account`: The numeric identifier for the account (long).
* `currency`: The currency in which the account operates.
* `balance`: The initial balance of the account.
* `overdraft`: The overdraft limit allowed for the account.

The `sendingTime` can be set using `SystemTimeProvider.CLOCK.currentTimeNanos()`.

==== Processing

Upon receiving a `CreateAccount` command:

1. Validate that `target` matches this instance's configured identifier.
2. Validate that `balance` ≥ 0.
3. Validate that the `account` number does not already exist in the system.
4. If validation fails, output a `createAccountFailed` event, including the reason.
5. If validation succeeds:

* Store the account details in a local data structure (e.g., a `Map<Long, CreateAccount>`).
* Emit an `onCreateAccount` event with the stored `CreateAccount` details.

==== Outputs

* `onCreateAccount` event on success:
* `sender`: System ID (e.g., `vault`)
* `target`: The original `sender` of the `CreateAccount` command
* `sendingTime`: The system's current time
* Embedded `createAccount` field containing the original request data.

* `createAccountFailed` event on failure:
* `sender`: System ID (e.g., `vault`)
* `target`: The original `sender` of the `CreateAccount` command
* `sendingTime`: The system's current time
* `reason`: A textual description of the failure (e.g., "invalid balance", "account already exists").

=== Transfer Funds

==== Inputs

1. A `Transfer` command containing:

* `sender`: The origin of the command.
* `target`: The intended system (e.g., `vault`).
* `sendingTime`: Holds when the command was sent as a wall clock timestamp with nanosecond resolution.
* `from`: The source account number.
* `to`: The destination account number.
* `currency`: The currency of the transfer.
* `amount`: The amount to transfer.
* `reference`: A reference field for the reason for the transfer or details.

==== Processing

Upon receiving a `Transfer` command:

1. Validate that `target` matches this instance's identifier.
2. Verify that the `from` account exists and its currency matches the `Transfer.currency`.
3. Verify that the `to` account exists and its currency matches the `Transfer.currency`.
4. Check that the `from` account has sufficient funds (`balance + overdraft ≥ amount`).
5. If validation fails, emit a `transferFailed` event with an appropriate `reason`.
6. If valid, update both accounts:
* Deduct the `amount` from the `from` account's balance.
* Add `amount` to the `to` account's balance.
7. Emit an `onTransfer` event indicating success.

==== Outputs

* `onTransfer` event on success:
* `sender`: System ID (e.g., `vault`)
* `target`: The original `sender` of the `Transfer` command
* `sendingTime`: The system's current time
* Embedded `transfer` field containing the original `Transfer` command data.

* `transferFailed` event on failure:
* `sender`: System ID (e.g., `vault`)
* `target`: The original `sender` of the `Transfer` command
* `sendingTime`: The system's current time
* Embedded `transfer` field containing the original request
* `reason`: A textual description of the failure (e.g., "from account doesn't exist", "insufficient funds").

=== Checkpoint

==== Inputs

1. A `CheckPoint` command containing:
* `sender`: The origin of the command.
* `target`: The intended system (e.g., `vault`).
* `sendingTime`: The timestamp of when the command was sent.

==== Processing

Upon receiving a `CheckPoint` command:

1. Validate that `target` matches this instance's identifier.
2. Emit a `startCheckpoint` event.
3. For every account currently held in the system:
* Emit an `onCreateAccount` event representing its current state.
4. Emit an `endCheckpoint` event.

==== Outputs

* `startCheckpoint` event:
* `sender`: The original `sender` of the `CheckPoint`
* `target`: The system ID (e.g., `vault`)
* `sendingTime`: The system's current time

* A series of `onCreateAccount` events for each known account, reflecting their current state at the time of checkpoint.

* `endCheckpoint` event:
* `sender`: The original `sender` of the `CheckPoint`
* `target`: The system ID (e.g., `vault`)
* `sendingTime`: The system's current time

== Non-Functional Requirements

1. **Performance**: The system should handle account lookups and updates in O(1) average time via efficient data structures (e.g., HashMap or LinkedHashMap).

2. **Concurrency**: The system may assume single-threaded inputs.

3. **Error Handling**: All invalid or unexpected command conditions result in failure events rather than exceptions visible to callers.

4. **Time Management**: `sendingTime` should be based on a reliable system clock.

== Validation and Testing

To verify these requirements:

1. Send a `createAccount` command with valid parameters and ensure `onCreateAccount` is emitted.
2. Send a `createAccount` command with invalid parameters (e.g., negative balance or duplicate account number) and confirm that `createAccountFailed` is emitted.
3. Perform a valid `transfer` and ensure `onTransfer` is emitted with the updated balances.
4. Attempt invalid transfers and ensure `transferFailed` events are emitted.
5. Issue a `checkPoint` command and validate that `startCheckpoint`, multiple `onCreateAccount` events (one per account), and `endCheckpoint` are produced in order.

== Traceability

Each requirement above directly corresponds to a portion of the Java implementation in `AccountManagerImpl.java`:

* Account creation logic: `createAccount(CreateAccount createAccount)`
* Transfer logic: `transfer(Transfer transfer)`
* Checkpoint logic: `checkPoint(CheckPoint checkPoint)`

Events and conditions are explicitly handled in private utility methods (e.g., `sendCreateAccountFailed`, `sendOnCreateAccount`, `sendTransferFailed`, `sendOnTransfer`).

== Conclusion

This document provides a high-level specification of the required functionalities and expected behaviours of the Account Management Service. Implementing these requirements should align with the Java code structure and produce consistent events for all supported operations.
Loading