Skip to content

Refactor app_config.rs: collapse the ~18 default_* functions and group fields, with a doc-coverage test to stop drift #1322

@lovasoa

Description

@lovasoa

Splitting the "app_config.rs is a large multi-responsibility module" item out of #1249 into a concrete, actionable refactor.

Problem

Adding a single config option to src/app_config.rs touches 5+ disconnected places, and nothing makes the compiler tell you when you forgot one:

  1. A new field on the ~30-field AppConfig struct, usually with its own #[serde(default = "...")].

  2. A standalone top-level default_*() free function returning a literal, declared far away from the field it belongs to. There are 18 of them today:

    $ grep -c "fn default_" src/app_config.rs
    18
    

    e.g. default_max_file_size, default_max_pending_rows, default_oidc_scopes.

  3. Sometimes a bespoke deserialize_with helper: deserialize_port, deserialize_socket_addr, deserialize_site_prefix, deserialize_duration_seconds.

  4. The env-var name is implicit (resolved by the config crate in load_from_file / env_config), so it never appears in code next to the field.

  5. A hand-maintained row in the configuration.md table. Nothing ties the field to its default to its doc row.

Concrete drift this already caused

The doc comment on max_uploaded_file_size disagrees with its own default function:

So the inline Rust comment drifted to claim 10 MiB while the real default and the docs table both say 5 MiB. Exactly the silent drift this structure invites.

Proposed change

  • Move the literal defaults into a single impl Default for AppConfig (or a const-backed table) and use #[serde(default)], eliminating the ~18 default_* free functions and putting each default next to its field.
  • Group the flat struct into nested domain sub-structs (database, oidc, https, telemetry, markdown) with #[serde(flatten)], so related field + default + validation co-locate and the file shrinks.
  • Add a doc-coverage test asserting every public config field name appears in configuration.md, killing drift mechanically (a value-coverage assertion could also catch the 5/10 MiB case).

Risks and mitigations

  • serde rename/flatten can change the accepted file keys / env var names. Mitigate by keeping field names byte-for-byte identical and adding round-trip tests that load representative existing config files / env layouts and assert the parsed AppConfig is unchanged.
  • Land the impl Default change separately from the sub-struct grouping, so each PR is small and the bisect surface stays tight.

Expected win

One place to edit per option, defaults next to their fields, and doc drift caught by a test instead of by a user.

Refs #1249.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions