Skip to content

refactor: remove from_globals() and construct AppContext explicitly (#656 PR F) - #671

Merged
grunch merged 1 commit into
cleanup/add-keys-to-context-656from
cleanup/remove-from-globals-656
Mar 18, 2026
Merged

refactor: remove from_globals() and construct AppContext explicitly (#656 PR F)#671
grunch merged 1 commit into
cleanup/add-keys-to-context-656from
cleanup/remove-from-globals-656

Conversation

@mostronatorcoder

@mostronatorcoder mostronatorcoder Bot commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

Context

Issue #656 tracks cleanup tasks after phase-5 DI migration (#639).

This PR implements PR F: remove the from_globals() bridge and construct AppContext explicitly at bootstrap.

⚠️ Note: This PR is based on PR E (#670). Merge PR E first.

Problem

AppContext::from_globals() was a transitional bridge between the old global-based architecture and the new DI pattern. Now that all handlers and scheduler use AppContext, we can eliminate this bridge.

Solution

Construct AppContext explicitly in main.rs using values already available:

  • get_db_pool() — database pool
  • client — Nostr client
  • MOSTRO_CONFIG — settings
  • MESSAGE_QUEUES.queue_order_msg — message queue
  • mostro_keys — signing keys

Changes

main.rs

  • Construct AppContext::new() explicitly with all dependencies
  • Pass ctx to run() instead of my_keys and client

app.rs

  • run() signature: (my_keys, client, ln_client)(ctx, ln_client)
  • Extract dependencies from ctx at function start
  • Remove from_globals() call from event loop

context.rs

  • Removed from_globals() method entirely 🗑️
  • Removed unused MESSAGE_QUEUES import

Breaking Changes

run() function signature changed:

// Before
pub async fn run(my_keys: Keys, client: &Client, ln_client: &mut LndConnector)

// After  
pub async fn run(ctx: AppContext, ln_client: &mut LndConnector)

Benefits

  1. No more bridge code — cleaner architecture
  2. Single construction pointAppContext built once in main
  3. Explicit dependencies — all deps visible at construction
  4. Testability — easier to mock in tests

Diff Stats

  • 3 files changed
  • +26 / -61 lines
  • Net: -35 lines 🧹

Validation

cargo fmt
cargo clippy --all-targets --all-features -- -D warnings
cargo test --bin mostrod (189 passed, 0 failed)

🎉 DI Migration Complete!

With this PR, the dependency injection migration from #639 is complete:

PR Description Status
PR A Remove legacy wrappers #663
PR B Simplify dispatcher #665
PR C Remove global accesses #666
PR D Migrate scheduler #667
PR E Add keys to AppContext #670
PR F Remove from_globals() 🔄 This PR

Related

Summary by CodeRabbit

  • Refactor
    • Simplified application initialization by consolidating configuration dependencies into a single context object, improving code maintainability and reducing complexity in the initialization flow.

…656 PR F)

## Context

Issue #656 tracks cleanup tasks after phase-5 DI migration (#639).
This PR implements **PR F**: remove the `from_globals()` bridge and
construct `AppContext` explicitly at bootstrap.

## Problem

`AppContext::from_globals()` was a transitional bridge between the old
global-based architecture and the new DI pattern. Now that all handlers
and scheduler use `AppContext`, we can eliminate this bridge.

## Solution

Construct `AppContext` explicitly in `main.rs` using values already
available at that point:
- `get_db_pool()` — database pool
- `client` — Nostr client
- `MOSTRO_CONFIG` — settings
- `MESSAGE_QUEUES.queue_order_msg` — message queue
- `mostro_keys` — signing keys

## Changes

### main.rs
- Construct `AppContext::new()` explicitly with all dependencies
- Pass `ctx` to `run()` instead of `my_keys` and `client`
- Import `MESSAGE_QUEUES` and `MOSTRO_CONFIG`

### app.rs
- `run()` signature changed: `(my_keys, client, ln_client)` → `(ctx, ln_client)`
- Extract `my_keys`, `client`, and `pow` from `ctx` at function start
- Remove `AppContext::from_globals()` call from event loop
- Remove unused `Settings` import

### context.rs
- **Removed `from_globals()` method entirely**
- Removed unused `MESSAGE_QUEUES` import

## Breaking Changes

`run()` function signature changed:
- **Before:** `run(my_keys: Keys, client: &Client, ln_client: &mut LndConnector)`
- **After:** `run(ctx: AppContext, ln_client: &mut LndConnector)`

## Benefits

1. **No more bridge code** — cleaner architecture
2. **Single construction point** — `AppContext` built once in main
3. **Explicit dependencies** — all deps visible at construction
4. **Testability** — easier to mock in tests

## Diff Stats

- 3 files changed
- +26 / -61 lines
- **Net: -35 lines** 🧹

## Validation

✅ `cargo fmt`
✅ `cargo clippy --all-targets --all-features -- -D warnings`
✅ `cargo test --bin mostrod` (189 passed, 0 failed)

## DI Migration Complete! 🎉

With this PR, the dependency injection migration from #639 is **complete**:

| PR | Description | Status |
|----|-------------|--------|
| PR A | Remove legacy wrappers | ✅ #663 |
| PR B | Simplify dispatcher | ✅ #665 |
| PR C | Remove global accesses | ✅ #666 |
| PR D | Migrate scheduler | ✅ #667 |
| PR E | Add keys to AppContext | ✅ #670 |
| **PR F** | Remove from_globals() | ✅ This PR |

## Related

- Parent cleanup issue: #656
- Suggested by @codaMW in #667 review

@arkanoider arkanoider left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK on my side!

@grunch

grunch commented Mar 18, 2026

Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 18, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b8573a76-7189-4e75-9a34-59714a5aafdb

📥 Commits

Reviewing files that changed from the base of the PR and between a3b6acc and 8e1191a.

📒 Files selected for processing (3)
  • src/app.rs
  • src/app/context.rs
  • src/main.rs
💤 Files with no reviewable changes (1)
  • src/app/context.rs

Walkthrough

This PR refactors the dependency injection pattern by consolidating scattered function parameters into a single AppContext argument. It removes the global-based AppContext::from_globals() constructor and updates call sites to explicitly construct and pass AppContext, shifting from implicit global state to explicit dependency passing at startup.

Changes

Cohort / File(s) Summary
AppContext Signature Refactor
src/app.rs, src/app/context.rs
Updated run() to accept AppContext instead of separate my_keys, client parameters; removed AppContext::from_globals() method, eliminating global-bridge initialization logic including nsec_privkey parsing and validation.
Dependency Wiring & Public API
src/main.rs
Replaced global-based AppContext construction with explicit AppContext::new() call; added public exports MESSAGE_QUEUES and MOSTRO_CONFIG from config; updated run() invocation to accept context and modified start_scheduler() to receive cloned context.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • arkanoider
  • Catrya

Poem

🐰 A refactor so clean, with contexts so bright,
Global state begone—explicit's the right!
Dependencies flow from main down the line,
No hidden globals, just AppContext divine. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor: remove from_globals() and construct AppContext explicitly' accurately summarizes the main changes—removing the from_globals() method and replacing it with explicit AppContext construction in main.rs, which is the primary objective of the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cleanup/remove-from-globals-656
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@grunch grunch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

@grunch
grunch merged commit 056fd88 into cleanup/add-keys-to-context-656 Mar 18, 2026
5 checks passed
@grunch
grunch deleted the cleanup/remove-from-globals-656 branch March 18, 2026 16:59
mostronatorcoder Bot added a commit that referenced this pull request Mar 18, 2026
…656 PR F) (#671)

## Context

Issue #656 tracks cleanup tasks after phase-5 DI migration (#639).
This PR implements **PR F**: remove the `from_globals()` bridge and
construct `AppContext` explicitly at bootstrap.

## Problem

`AppContext::from_globals()` was a transitional bridge between the old
global-based architecture and the new DI pattern. Now that all handlers
and scheduler use `AppContext`, we can eliminate this bridge.

## Solution

Construct `AppContext` explicitly in `main.rs` using values already
available at that point:
- `get_db_pool()` — database pool
- `client` — Nostr client
- `MOSTRO_CONFIG` — settings
- `MESSAGE_QUEUES.queue_order_msg` — message queue
- `mostro_keys` — signing keys

## Changes

### main.rs
- Construct `AppContext::new()` explicitly with all dependencies
- Pass `ctx` to `run()` instead of `my_keys` and `client`
- Import `MESSAGE_QUEUES` and `MOSTRO_CONFIG`

### app.rs
- `run()` signature changed: `(my_keys, client, ln_client)` → `(ctx, ln_client)`
- Extract `my_keys`, `client`, and `pow` from `ctx` at function start
- Remove `AppContext::from_globals()` call from event loop
- Remove unused `Settings` import

### context.rs
- **Removed `from_globals()` method entirely**
- Removed unused `MESSAGE_QUEUES` import

## Breaking Changes

`run()` function signature changed:
- **Before:** `run(my_keys: Keys, client: &Client, ln_client: &mut LndConnector)`
- **After:** `run(ctx: AppContext, ln_client: &mut LndConnector)`

## Benefits

1. **No more bridge code** — cleaner architecture
2. **Single construction point** — `AppContext` built once in main
3. **Explicit dependencies** — all deps visible at construction
4. **Testability** — easier to mock in tests

## Diff Stats

- 3 files changed
- +26 / -61 lines
- **Net: -35 lines** 🧹

## Validation

✅ `cargo fmt`
✅ `cargo clippy --all-targets --all-features -- -D warnings`
✅ `cargo test --bin mostrod` (189 passed, 0 failed)

## DI Migration Complete! 🎉

With this PR, the dependency injection migration from #639 is **complete**:

| PR | Description | Status |
|----|-------------|--------|
| PR A | Remove legacy wrappers | ✅ #663 |
| PR B | Simplify dispatcher | ✅ #665 |
| PR C | Remove global accesses | ✅ #666 |
| PR D | Migrate scheduler | ✅ #667 |
| PR E | Add keys to AppContext | ✅ #670 |
| **PR F** | Remove from_globals() | ✅ This PR |

## Related

- Parent cleanup issue: #656
- Suggested by @codaMW in #667 review

Co-authored-by: MostronatorCoder[bot] <182182091+MostronatorCoder[bot]@users.noreply.github.com>
grunch pushed a commit that referenced this pull request Mar 18, 2026
…656 PR F) (#671) (#672)

## Context

Issue #656 tracks cleanup tasks after phase-5 DI migration (#639).
This PR implements **PR F**: remove the `from_globals()` bridge and
construct `AppContext` explicitly at bootstrap.

## Problem

`AppContext::from_globals()` was a transitional bridge between the old
global-based architecture and the new DI pattern. Now that all handlers
and scheduler use `AppContext`, we can eliminate this bridge.

## Solution

Construct `AppContext` explicitly in `main.rs` using values already
available at that point:
- `get_db_pool()` — database pool
- `client` — Nostr client
- `MOSTRO_CONFIG` — settings
- `MESSAGE_QUEUES.queue_order_msg` — message queue
- `mostro_keys` — signing keys

## Changes

### main.rs
- Construct `AppContext::new()` explicitly with all dependencies
- Pass `ctx` to `run()` instead of `my_keys` and `client`
- Import `MESSAGE_QUEUES` and `MOSTRO_CONFIG`

### app.rs
- `run()` signature changed: `(my_keys, client, ln_client)` → `(ctx, ln_client)`
- Extract `my_keys`, `client`, and `pow` from `ctx` at function start
- Remove `AppContext::from_globals()` call from event loop
- Remove unused `Settings` import

### context.rs
- **Removed `from_globals()` method entirely**
- Removed unused `MESSAGE_QUEUES` import

## Breaking Changes

`run()` function signature changed:
- **Before:** `run(my_keys: Keys, client: &Client, ln_client: &mut LndConnector)`
- **After:** `run(ctx: AppContext, ln_client: &mut LndConnector)`

## Benefits

1. **No more bridge code** — cleaner architecture
2. **Single construction point** — `AppContext` built once in main
3. **Explicit dependencies** — all deps visible at construction
4. **Testability** — easier to mock in tests

## Diff Stats

- 3 files changed
- +26 / -61 lines
- **Net: -35 lines** 🧹

## Validation

✅ `cargo fmt`
✅ `cargo clippy --all-targets --all-features -- -D warnings`
✅ `cargo test --bin mostrod` (189 passed, 0 failed)

## DI Migration Complete! 🎉

With this PR, the dependency injection migration from #639 is **complete**:

| PR | Description | Status |
|----|-------------|--------|
| PR A | Remove legacy wrappers | ✅ #663 |
| PR B | Simplify dispatcher | ✅ #665 |
| PR C | Remove global accesses | ✅ #666 |
| PR D | Migrate scheduler | ✅ #667 |
| PR E | Add keys to AppContext | ✅ #670 |
| **PR F** | Remove from_globals() | ✅ This PR |

## Related

- Parent cleanup issue: #656
- Suggested by @codaMW in #667 review

Co-authored-by: mostronatorcoder[bot] <263173566+mostronatorcoder[bot]@users.noreply.github.com>
Co-authored-by: MostronatorCoder[bot] <182182091+MostronatorCoder[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants