Skip to content

Fix latest refactor bug #12

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

Merged
merged 57 commits into from
Jul 6, 2025
Merged

Fix latest refactor bug #12

merged 57 commits into from
Jul 6, 2025

Conversation

fulleni
Copy link
Member

@fulleni fulleni commented Jul 6, 2025

Status

READY/IN DEVELOPMENT/HOLD

Description

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

fulleni added 30 commits July 6, 2025 17:55
The Uuid provider was previously injected in `server.dart` after the root middleware from `routes/_middleware.dart` was composed. This caused a `Bad state` error because the middleware attempted to read the Uuid from the context before it was provided.

This change moves the Uuid provider to the beginning of the middleware chain in `routes/_middleware.dart`, ensuring it is available for all subsequent middleware and route handlers that depend on it for generating request IDs. The redundant provider has been removed from `server.dart`.
The Uuid provider was previously injected in `server.dart` after the root middleware from `routes/_middleware.dart` was composed. This caused a `Bad state` error because the middleware attempted to read the Uuid from the context before it was provided.

This change moves the Uuid provider to the beginning of the middleware chain in `routes/_middleware.dart`, ensuring it is available for all subsequent middleware and route handlers that depend on it for generating request IDs. The redundant provider has been removed from `server.dart`.
Replaces the static CORS configuration with a dynamic, per-request policy.

This new middleware is production-ready and developer-friendly:
- In production, it strictly enforces the origin specified in the `CORS_ALLOWED_ORIGIN` environment variable.
- In development (if the variable is not set), it inspects the request's `Origin` header and automatically allows any request from `localhost` on any port.

This fixes issues with the Flutter web dev server using random ports and removes the need for manual configuration during local development.
Updated the "Note on Web Client Integration (CORS)" section to reflect the new dynamic CORS handling for local development. The documentation now correctly states that `localhost` origins are automatically allowed on any port during development, and the `CORS_ALLOWED_ORIGIN` variable is only required for production environments.
The middleware chain in `routes/_middleware.dart` was ordered incorrectly, causing the `RequestId` provider to execute before the `Uuid` provider. This resulted in a `Bad state: context.read<Uuid>()` error on every request.

This change reorders the `.use()` calls to ensure the `Uuid` provider is applied to the context first, making it available to the `RequestId` provider and resolving the crash.
The previous implementation of the `/api/v1` middleware used a higher-order function that created an isolated handler chain, breaking the flow of the request context from `server.dart`. This caused `Bad state` errors when the authentication middleware tried to read services like `AuthTokenService`.

This change refactors the middleware to use a standard `.use()` chain. It now leverages the `originChecker` callback from `shelf_cors_headers` to handle dynamic CORS policies correctly within a standard composition pattern. This ensures the request context is preserved, and all downstream middleware can access the globally provided services.
Introduces a singleton `DependencyContainer` to act as a centralized service locator. This is a crucial architectural change to fix dependency injection issues caused by the Dart Frog middleware lifecycle.

- The container is initialized once at startup in `server.dart`.
- The root middleware will use it to provide all dependencies to the request context.
- Includes extensive documentation explaining the rationale and fixing the "context.read() called too early" problem.
This change modifies the server startup process to populate the newly created DependencyContainer singleton with all initialized repositories and services. It removes the old, ineffective provider chain that was causing dependency injection failures due to the Dart Frog middleware lifecycle.
Refactors the root middleware to provide all shared repositories and services to the request context. It reads the initialized instances from the `DependencyContainer` singleton, which is populated at startup.

This change completes the architectural refactoring to a centralized dependency injection model, ensuring that all dependencies are available to downstream middleware and route handlers, finally resolving the "context.read() called too early" errors.
Refactors the server startup logic into a custom `bin/server.dart` entrypoint. This resolves a `LateInitializationError` caused by a race condition where requests could be processed before the asynchronous dependency initialization was complete.

- All async setup (DB connection, seeding, service creation) is now completed and `await`ed in `main()`.
- The `DependencyContainer` is populated only after all services are ready.
- The Dart Frog request handler is built and served only after all initialization is finished.
- Deletes the old, now-redundant `lib/src/config/server.dart`.
Implements an initialization gate using a `Completer` in `bin/server.dart` to prevent a race condition where requests could be processed before asynchronous setup (DB connection, dependency injection) was complete.

- A new `_initializationGate` middleware now wraps the root handler.
- This middleware awaits a `Completer`'s future, effectively pausing all incoming requests.
- The completer is only marked as complete after all async initialization in `main()` is finished and the server is listening.
- This guarantees that the `DependencyContainer` is fully populated before any request attempts to access it, resolving the `LateInitializationError`.
Refactors the server startup logic into a custom `run` function in `lib/src/config/server.dart` to align with the `dart_frog dev` lifecycle.

This change resolves a `LateInitializationError` caused by a race condition where requests could be processed before asynchronous dependency setup was complete.

- All async setup (DB connection, seeding, service creation) is now completed within the `run` function.
- A `Completer`-based "initialization gate" middleware is used to hold all incoming requests until the server is fully initialized and the `DependencyContainer` is populated.
- Deletes the unused `bin/server.dart` file.
Improves code clarity and robustness in `server.dart` by explicitly typing service variables with their abstract base classes (e.g., `AuthTokenService`) instead of relying on type inference for their concrete implementations. This makes dependency contracts clearer and aligns with best practices.
Instruments the `run` function in `lib/server.dart` with detailed, sequential log messages. This provides clear visibility into the server startup process, from logger setup and database connection to dependency creation and the opening of the request gate. This will aid in debugging initialization-related issues.
Adds a log message to the `init` method of the `DependencyContainer`. This provides an explicit log entry confirming that the container has been successfully populated with all service and repository instances during the server startup sequence.
Instruments the root middleware (`routes/_middleware.dart`) with logging to trace the start of the request lifecycle. This includes logging the generation of a unique `RequestId` for each incoming request, improving debuggability and traceability.
Instruments the API v1 middleware (`routes/api/v1/_middleware.dart`) with logging. This adds visibility into the CORS origin checking logic and confirms when a request enters the CORS and authentication middleware handlers, completing the request lifecycle trace.
Refactors the dependency injection mechanism to be handled entirely within the root middleware (`routes/_middleware.dart`).

- All services and repositories are now initialized asynchronously within the middleware for each request.
- Uses the `DatabaseConnectionManager` singleton to ensure a single, shared database connection.
- This approach is more robust and aligned with Dart Frog's lifecycle, removing the reliance on a custom `run` function and the `DependencyContainer` singleton.
Introduces `DatabaseConnectionManager`, a singleton class to manage a single, shared PostgreSQL connection for the application. This ensures the database is connected only once, improving performance and resource management. It uses a `Completer` to make the connection available asynchronously and includes logging for the initialization process.
Introduces `AppDependencies`, a singleton class to manage the lifecycle of all application services and repositories.

This class uses a lazy initialization pattern, creating and configuring all dependencies only on the first request. It leverages the `DatabaseConnectionManager` and includes detailed logging to trace the initialization process, ensuring a robust and performant startup sequence.
Refactors the root middleware to use the new `AppDependencies` singleton for dependency injection.

- The middleware now triggers a single, idempotent `init()` call on the `AppDependencies` instance.
- It then provides all the pre-initialized services and repositories from the singleton into the request context.
- This simplifies the middleware, improves performance by ensuring dependencies are created only once, and creates a more robust and maintainable initialization flow.
Deletes `lib/server.dart` and `lib/src/config/dependency_container.dart`.

These files are no longer needed after refactoring the application to use a lazy-initialized, singleton-based dependency injection pattern triggered by the root middleware. This completes the transition to a more robust and standard Dart Frog architecture.
Re-introduces the root logger configuration, which was lost during the refactor away from `lib/server.dart`.

The configuration is now placed in the root middleware and uses a flag to ensure it only runs once per application lifecycle. This is critical for enabling log output for all subsequent processes, including dependency initialization and error handling.
Enhances `EnvironmentConfig` to log all available environment variables if `DATABASE_URL` is not found. This provides crucial diagnostic information to debug configuration issues where environment variables are not being loaded as expected.
Adds the `dotenv` package to manage environment variables directly within the application. This makes the server's configuration self-contained and less reliant on the execution environment of the `dart_frog_cli` tool.
fulleni added 26 commits July 6, 2025 20:44
Updates the `dotenv` usage to correctly instantiate the `DotEnv` class and call the `load()` method on the instance, as per the package's documentation. This ensures that environment variables from the .env file are properly loaded into the application's environment.
Refactors environment variable handling to be self-contained within the `EnvironmentConfig` class.

- The `dotenv` loading logic is moved from `AppDependencies` to `EnvironmentConfig`.
- `EnvironmentConfig` now creates a static `DotEnv` instance, loads the `.env` file, and all getters now read from this instance instead of `Platform.environment`.
- This ensures that environment variables are loaded and read correctly from a single, reliable source, resolving the issue where variables were not available to the application.
Refactors environment variable handling to be self-contained within the `EnvironmentConfig` class.

- `EnvironmentConfig` now creates a static `DotEnv` instance, loads the `.env` file, and all getters now read from this instance instead of `Platform.environment`.
- This ensures that environment variables are loaded and read correctly from a single, reliable source, resolving the issue where variables were not available to the application.
- Updates the `CREATE TABLE` statement for the `categories` table to include `slug`, `description`, `status`, and `type` columns.
- Updates the corresponding `INSERT` statement in the seeding logic to populate all fields from the `Category` model.
- Changes the `ON CONFLICT` clause to use the `slug` as the unique business key for idempotency.

This resolves the "superfluous variables" error during database seeding.
The `slug` field is not used by the client applications and was causing a mismatch between the `Category` data model and the database schema, leading to a server crash on startup.

This change removes the `slug` column from the `categories` table definition and updates the database seeder to no longer attempt to insert a slug. The `ON CONFLICT` clause for seeding has been updated to use the primary key `id` to ensure idempotency.
The `categories` table schema in the database seeder was missing the `icon_url` column, which is present in the `Category` data model. This caused a mismatch between the model and the database layer.

This change adds the `icon_url` column to the `CREATE TABLE` and `INSERT` statements in the `DatabaseSeedingService`, ensuring the schema is fully synchronized with the model.
            Sql.named(
              'INSERT INTO categories (id, name, description, icon_url, '
              'status, type, created_at, updated_at) VALUES (@id, @name, @description, '
              '@icon_url, @status, @type, @created_at, @updated_at) '
              'ON CONFLICT (id) DO NOTHING',
            ),
            parameters: category.toJson(),
            Sql.named(
              'INSERT INTO headlines (id, title, source_id, category_id, '
              'image_url, url, published_at, description, content) '
              'VALUES (@id, @title, @source_id, @category_id, @image_url, @url, '
              '@published_at, @description, @content) '
              'ON CONFLICT (id) DO NOTHING',
            ),
            parameters: headline.toJson(),
The database seeding process was crashing with a "Missing variable" error. This was caused by a conflict between the models' `toJson` method (which uses `includeIfNull: false` and omits keys for null values) and the `postgres` driver's requirement that all named SQL parameters exist in the provided parameters map.

This change refactors the seeding logic to manually ensure that parameter maps for `categories` and `headlines` always contain keys for all optional/nullable columns (e.g., `description`, `icon_url`, `content`), setting their value to `null` if they are not already present. This makes the seeding process robust against incomplete fixture data and permanently resolves the startup error.
The `sources` table in the database was overly simplistic and did not reflect the rich `Source` data model, leading to data loss and seeding errors.

This change expands the `sources` table schema to include all fields from the `Source` model, such as `description`, `url`, and `status`. It correctly handles nested objects by storing `source_type` as `JSONB` and creating a foreign key relationship for `headquarters` to the `countries` table. The seeding logic has been updated to populate these new columns, resolving the architectural mismatch.
The `user_app_settings` table incorrectly defined the `language` column as `JSONB`, while the `UserAppSettings` model correctly defines it as a `String`. This caused a crash during database seeding when trying to call `.toJson()` on a String.

This change corrects the `language` column type to `TEXT` in the schema and updates the seeding logic to pass the language as a simple string, aligning the database with the data model and resolving the startup error.
…arams

The `sources` seeder was failing with a "superfluous variables" error for the `headquarters` key. The logic correctly added the `headquarters_country_id` to the parameter map but failed to remove the original nested `headquarters` object.

This change updates the seeder to remove the `headquarters` key from the map after its ID has been extracted, ensuring the parameter map exactly matches the SQL query and resolving the startup error.
The database seeding was failing with a JSON syntax error because the `sources` table defined the `source_type` column as `JSONB`, but the `Source` model provides this value as a simple string (from an enum).

This change corrects the `source_type` column type to `TEXT`, aligning the database schema with the data model and resolving the final seeding error.
The database seeding was failing with a foreign key constraint violation because it attempted to seed `sources` before `countries`. The `sources` table has a foreign key dependency on the `countries` table.

This change reorders the operations in `seedGlobalFixtureData`, ensuring that `countries` are seeded before `sources`, which resolves the error.
The `countries` table schema was not synchronized with the `Country` model, using an incorrect `code` column and missing `flag_url`, `status`, and `type`. This caused a crash during database seeding.

This change updates the `countries` table schema and its corresponding seeding logic to perfectly mirror the `Country` model. This resolves the final schema mismatch and allows the server to start successfully.
The database seeding was crashing with a "Missing variable for 'source_id'" error due to invalid fixture data. Additionally, the `INSERT` statement for headlines was missing the `created_at` and `updated_at` columns.

This change makes the headline seeder more robust by validating that required fields (`source_id`, `category_id`) are present before attempting insertion, skipping invalid fixtures with a warning. It also updates the `INSERT` statement to be fully aligned with the table schema, resolving all remaining seeding issues.
The `headlines` table schema was missing the `status` and `type` columns inherited from its `FeedItem` base class. Additionally, the seeding logic was not correctly handling the nested `source` and `category` objects present in the `Headline` model, leading to a crash.

This change updates the `headlines` table schema to be a complete representation of the model. The seeding logic is refactored to correctly extract foreign key IDs from the nested objects and to ensure the parameter map sent to the database driver is perfectly aligned with the schema. This resolves the final seeding error.
The server was failing to start due to a syntax error (a missing comma) in the `CREATE TABLE` statement for the `headlines` table.

Additionally, the schema incorrectly defined the `image_url`, `url`, and `published_at` columns as `NOT NULL`, which contradicted the nullable fields in the `Headline` data model.

This change corrects the syntax error and removes the incorrect `NOT NULL` constraints, fully aligning the database schema with the model and resolving the startup failure.
The `headlines` table schema incorrectly contained a `content` column not present in the `Headline` model. Additionally, the seeder logic was not robustly handling all nullable fields (like `image_url`), causing a "Missing variable" crash.

This change removes the `content` column from the schema and `INSERT` statement. It also updates the seeder to ensure all optional fields from the model are present in the parameter map. This fully aligns the database layer with the data model and resolves the final startup error.
The admin user seeding was failing with a JSON syntax error. This was caused by passing raw lists of complex objects (e.g., `List<Category>`) to the database driver for `JSONB` columns, which the driver cannot serialize automatically.

This change updates the seeding logic to use the `UserContentPreferences.toJson()` method, which correctly serializes the data into a JSON-compatible format before insertion. This resolves the final startup error.
The admin user seeding was failing with a JSON syntax error because the `roles` list (`List<String>`) was being passed directly to the database driver for a `JSONB` column. The driver did not correctly serialize this into a valid JSON array string.

This change uses `jsonEncode` to explicitly convert the `roles` list into a correctly formatted JSON string before insertion, resolving the final startup error.
User creation was failing with a JSON syntax error because the `roles` field (`List<String>`) was not being correctly serialized into a JSON array string for the database's `JSONB` column.

This change updates the `userRepository`'s configuration to use a custom `toJson` function. This function explicitly JSON-encodes the `roles` list before it's sent to the database client, resolving the error at the source without altering the generic repository or the User model.
Anonymous user creation was failing with a type cast error because the `postgres` driver returns native `DateTime` objects, while the `User.fromJson` factory expects ISO 8601 strings for date fields.

This change introduces a custom deserialization function for the `userRepository`. This function intercepts the data map from the database, converts `DateTime` objects to the expected string format, and then passes the corrected map to the standard `User.fromJson` factory. This resolves the runtime error by aligning the data format from the data source with the expectations of the data model.
The user_app_settings table was missing the feed_preferences, engagement_shown_counts, and engagement_last_shown_timestamps columns, causing a crash during new user creation.

This change updates the table schema and its corresponding seeding logic to be in perfect sync with the UserAppSettings model, resolving the runtime error.
User creation was failing with a type cast error when creating the default UserContentPreferences. This was caused by two issues:
1. Complex list objects were not being JSON-encoded before being sent to the database's JSONB columns.
2. DateTime objects returned from the database were not being converted to strings before being passed to the fromJson factory.

This change introduces custom toJson and fromJson functions for the userContentPreferencesRepository. These functions handle the necessary data transformations, ensuring correct serialization and deserialization and resolving the runtime error.
The generic data routes were failing with a "Bad state" error because the `ModelRegistryMap` was not being provided to the request context.

This change adds the `ModelRegistryMap` to the root middleware's dependency providers, making it available globally and resolving the error in the data route handlers.
The generic data routes were failing with a type cast error (`DateTime` is not a subtype of `String?`) when reading from the database. This was because the `postgres` driver returns native `DateTime` objects, while the models' `fromJson` factories expect ISO 8601 strings.

This change implements custom `fromJson` functions for all relevant repositories (`headline`, `category`, `source`, `country`, `appConfig`, `userAppSettings`). These functions pre-process the data from the database, converting `DateTime` objects to the expected string format before deserialization.

Additionally, it corrects the `toJson` logic for the `userAppSettingsRepository` to properly JSON-encode its complex fields for `JSONB` columns. This resolves all known data serialization and deserialization errors.
@fulleni fulleni merged commit bf23c06 into main Jul 6, 2025
1 check failed
@fulleni fulleni deleted the fix_latest_refactor_bug branch July 6, 2025 22:16
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @fulleni, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request delivers crucial fixes to address stability and data integrity issues stemming from a recent refactor. It introduces a centralized dependency injection system, significantly improves how complex data types are handled during database interactions, and enhances environment variable loading and CORS configuration. These changes collectively bolster the API's foundation, ensuring more reliable operation and easier future development.

Highlights

  • Centralized Dependency Management: The application's services and repositories are now managed and provided through a new AppDependencies singleton, ensuring consistent and efficient resource utilization across the API.
  • Improved Database Data Handling: Explicit serialization and deserialization logic has been implemented for complex data types (like DateTime and JSONB) within the PostgreSQL data layer, resolving issues with data persistence and retrieval.
  • Enhanced Environment Configuration: Environment variable loading has been migrated to the dotenv package, providing more robust configuration management and improved debugging capabilities for missing critical variables.
  • Refined CORS Behavior: The CORS middleware now dynamically handles localhost origins for development environments while maintaining strict CORS_ALLOWED_ORIGIN enforcement for production, with added logging for clarity.
  • Database Schema and Seeding Updates: Database table schemas for several core entities (categories, sources, countries, headlines, user_app_settings) have been updated to support richer data models, accompanied by revised seeding logic to correctly populate these new fields.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The code changes introduce a centralized AppDependencies class, improving structure and maintainability. Updates to CORS handling and environment variable loading are also solid improvements. However, there's a critical issue in the database seeding logic that could cause runtime failures due to case mismatches between JSON keys and SQL parameters.

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.

1 participant