Skip to content

chore(deps): update SpringUserFramework to 4.0.2#49

Merged
devondragon merged 3 commits into
mainfrom
feature/spring-user-framework-4.0.2-snapshot
Jan 25, 2026
Merged

chore(deps): update SpringUserFramework to 4.0.2#49
devondragon merged 3 commits into
mainfrom
feature/spring-user-framework-4.0.2-snapshot

Conversation

@devondragon

Copy link
Copy Markdown
Owner

Summary

  • Update ds-spring-user-framework from 4.0.1 to 4.0.2
  • Update Spring Boot from 4.0.1 to 4.0.2
  • Update Selenide from 7.13.0 to 7.14.0
  • Add Docker Compose support for local development with MariaDB
  • Add sample event data for local development testing

Test plan

  • All existing tests pass (116 passed, 0 failed)
  • Verify application starts with ./gradlew bootRun
  • Verify Docker Compose integration works with docker compose up

Update the ds-spring-user-framework dependency from 4.0.1 to 4.0.2
and add Docker Compose support for local development.

- Bump ds-spring-user-framework from 4.0.1 to 4.0.2
- Bump Spring Boot from 4.0.1 to 4.0.2
- Bump Selenide from 7.13.0 to 7.14.0
- Add spring-boot-docker-compose for local development
- Add compose.yaml with MariaDB service for local development
Add SQL script with sample event data to populate the database
during local development and testing.

- 15 sample events spanning 2025-2027
- Uses INSERT IGNORE to prevent duplicates on re-run
Copilot AI review requested due to automatic review settings January 25, 2026 22:23
@claude

claude Bot commented Jan 25, 2026

Copy link
Copy Markdown

Pull Request Review

I've reviewed PR #49 and have the following feedback:

✅ Positives

  1. Good Dependency Updates: The updates to Spring Boot 4.0.2, Spring User Framework 4.0.2, and Selenide 7.14.0 are appropriate patch version updates that should provide bug fixes and improvements.

  2. Docker Compose Integration: Adding spring-boot-docker-compose support is excellent for local development. The compose.yaml configuration is clean and follows best practices with named volumes for data persistence.

  3. Test Data for Local Development: The data-local.sql file provides good sample events for testing, making it easier for developers to work with the application locally.

  4. Clean Changes: The PR is focused and scoped appropriately - dependency updates plus related development improvements.

⚠️ Issues & Concerns

1. SQL Data File Won't Load Automatically (High Priority)

The data-local.sql file you've added won't be automatically loaded by Spring Boot because:

  • There's no spring.sql.init.mode configuration in any profile
  • Spring Boot looks for data.sql by default, not data-local.sql
  • No profile-specific configuration exists for the local profile to reference this file

Solutions:

  • Rename the file to data.sql (loaded by default), OR
  • Create src/main/resources/application-local.yml with:
    spring:
      sql:
        init:
          mode: always
          data-locations: classpath:data-local.sql

2. @FutureOrPresent Validation Conflict (Medium Priority)

In Event.java:34, the date field has @FutureOrPresent validation, but your sample data includes events with past dates (e.g., 'Tech Conference 2025' on 2025-06-15, which is already in the past relative to today's date 2026-01-25). This will cause validation failures if:

  • The data is loaded and then edited
  • Tests try to work with these events
  • Any application logic validates existing events

Solutions:

  • Update the sample event dates to be in the future (2026-2027+), OR
  • Remove the @FutureOrPresent constraint for existing events (consider only validating on creation/update), OR
  • Use INSERT IGNORE might not help if validation happens at the JPA layer before persistence

3. Security: Hardcoded Database Credentials (Medium Priority)

The compose.yaml contains hardcoded credentials:

MARIADB_USER: springuser
MARIADB_PASSWORD: springuser
MARIADB_ROOT_PASSWORD: rootpassword

While this is acceptable for local development, consider:

  • Adding a comment in compose.yaml warning that these are for local dev only
  • Documenting in the PR description or README that production deployments must use secure credentials
  • Consider using environment variable placeholders for better practices demonstration

4. INSERT IGNORE is MySQL/MariaDB Specific (Low Priority)

The SQL syntax INSERT IGNORE in data-local.sql:4 is specific to MySQL/MariaDB. According to build.gradle:64, the project also supports PostgreSQL (runtimeOnly 'org.postgresql:postgresql'). This will cause errors if someone runs with PostgreSQL.

Solutions:

  • Document that data-local.sql is MariaDB-specific, OR
  • Use standard SQL with conditional logic, OR
  • Provide separate data files for different databases

📋 Recommendations

  1. Update the PR Test Plan: Mark the unchecked items once you've verified them, or remove them if not applicable.

  2. Document Docker Compose Usage: Consider adding a section to the README explaining:

    • How Spring Boot auto-detects compose.yaml
    • What the Docker Compose integration provides
    • How to disable it if needed (spring.docker.compose.enabled=false)
  3. Consider Adding More Environment Variables: The current compose.yaml could expose the database port and credentials as environment variables for easier customization without editing the file.

🔒 Security Assessment

No critical security issues identified. The hardcoded credentials are acceptable for local development but should be documented as such.

⚡ Performance Considerations

No performance concerns. The dependency updates should maintain or improve performance.

🧪 Test Coverage

The PR mentions all tests pass (116 passed), which is excellent. However:

  • Consider adding integration tests for Docker Compose startup
  • Verify that the sample data loads correctly in the local profile

📊 Overall Assessment

This is a solid maintenance PR with useful local development improvements. The main blocker is that the SQL data file won't load without configuration changes. Once that's addressed, this will be ready to merge.

Recommendation: Request changes to fix the data loading issue before merging.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request updates several dependencies and adds local development infrastructure to the Spring User Framework demo application. The main changes include upgrading the Spring User Framework and Spring Boot to version 4.0.2, updating Selenide test framework, and introducing Docker Compose support with sample event data for easier local development setup.

Changes:

  • Dependency updates: Spring Boot (4.0.1 → 4.0.2), Spring User Framework (4.0.1 → 4.0.2), and Selenide (7.13.0 → 7.14.0)
  • New Docker Compose configuration for simplified local MariaDB setup
  • Sample event data SQL file for local development testing

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.

File Description
build.gradle Updated Spring Boot, Spring User Framework, and Selenide versions; added Docker Compose development dependency
compose.yaml New Docker Compose configuration for local MariaDB database setup
src/main/resources/data-local.sql Added 15 sample events for local development testing

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +6 to +7
(1, 'Tech Conference 2025', 'A conference about the latest in tech.', 'San Francisco, CA', '2025-06-15', '10:00:00'),
(2, 'Spring Boot Workshop', 'Learn Spring Boot from experts.', 'New York, NY', '2025-07-10', '14:00:00'),

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

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

The Event entity has a @FutureOrPresent validation constraint on the date field (see Event.java line 34), which requires dates to be today or in the future. The events with 2025 dates (rows 1-2) will fail this validation when the data is loaded, potentially causing the application startup to fail or these records to be rejected. Update these dates to be in 2026 or later to comply with the validation constraint.

Suggested change
(1, 'Tech Conference 2025', 'A conference about the latest in tech.', 'San Francisco, CA', '2025-06-15', '10:00:00'),
(2, 'Spring Boot Workshop', 'Learn Spring Boot from experts.', 'New York, NY', '2025-07-10', '14:00:00'),
(1, 'Tech Conference 2025', 'A conference about the latest in tech.', 'San Francisco, CA', '2026-06-15', '10:00:00'),
(2, 'Spring Boot Workshop', 'Learn Spring Boot from experts.', 'New York, NY', '2026-07-10', '14:00:00'),

Copilot uses AI. Check for mistakes.
Comment thread compose.yaml
Comment on lines +4 to +8
environment:
MARIADB_DATABASE: springuser
MARIADB_USER: springuser
MARIADB_PASSWORD: springuser
MARIADB_ROOT_PASSWORD: rootpassword

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

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

The compose.yaml uses MARIADB_* environment variables while the existing docker-compose.yml uses MYSQL_* environment variables (lines 9-13). Both MariaDB images support both naming conventions, but for consistency with the existing docker-compose.yml in this repository, consider using the MYSQL_* variable names instead. This makes it easier to maintain both files and reduces confusion.

Copilot uses AI. Check for mistakes.
Comment thread compose.yaml
@@ -0,0 +1,15 @@
services:
mariadb:
image: mariadb:11.6

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

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

The MariaDB image version in this new compose.yaml file is '11.6', while the existing docker-compose.yml file uses '11.6.2' (a more specific patch version). For consistency and to ensure reproducible builds, consider using the same specific version tag '11.6.2' instead of the major.minor version '11.6'. This prevents unexpected updates when the image is pulled.

Copilot uses AI. Check for mistakes.
Comment thread compose.yaml
Comment on lines +2 to +12
mariadb:
image: mariadb:11.6
environment:
MARIADB_DATABASE: springuser
MARIADB_USER: springuser
MARIADB_PASSWORD: springuser
MARIADB_ROOT_PASSWORD: rootpassword
ports:
- "3306:3306"
volumes:
- mariadb-data:/var/lib/mysql

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

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

The new compose.yaml file lacks health checks that are present in the existing docker-compose.yml (lines 16-22). Health checks are important for ensuring the database is fully initialized before the application attempts to connect. Consider adding a healthcheck configuration similar to the existing docker-compose.yml to improve reliability during startup.

Copilot uses AI. Check for mistakes.
- Remove @FutureOrPresent validation from Event entity to allow
  historical events and ensure sample data loads regardless of date
- Add security warning to compose.yaml clarifying credentials are
  for local development only
- Add MariaDB-specific syntax note to data-local.sql
@devondragon devondragon merged commit 6b45e09 into main Jan 25, 2026
6 checks passed
@claude

claude Bot commented Jan 25, 2026

Copy link
Copy Markdown

Pull Request Review

Thank you for this PR! Overall, this is a solid dependency update and developer experience improvement. Here's my detailed feedback:

Strengths

  1. Dependency Updates: Clean updates to Spring Boot (4.0.1 → 4.0.2), Spring User Framework (4.0.1 → 4.0.2), and Selenide (7.13.0 → 7.14.0)
  2. Docker Compose Integration: Excellent addition for local development with proper security warnings
  3. Sample Data: Well-structured test data with good variety of events

🔍 Issues & Concerns

1. CRITICAL: Breaking Change in Event Validation (src/main/java/com/digitalsanctuary/spring/demo/event/Event.java:32-33)

Issue: Removed @FutureOrPresent validation constraint without explanation or migration strategy.

-    @FutureOrPresent(message = "Event date must be today or in the future")
     private LocalDate date;

Impact:

  • Allows creating events with past dates, which may not align with business requirements
  • Tests still create future dates (LocalDate.now().plusDays(30)), suggesting future dates were intentional
  • The sample data in data-local.sql contains hardcoded future dates (2025-2027), which will become past dates over time

Recommendation:

  • If allowing past events is intentional (for historical records), document this in the PR description and consider adding a field like isPastEvent or status
  • If this was removed to accommodate the sample data, consider using dynamic dates in SQL or keeping the validation
  • Update the validation message or add application-level logic to handle historical events appropriately

2. SQL Data Initialization Not Configured (src/main/resources/data-local.sql)

Issue: Created data-local.sql but there's no configuration to load it.

Missing Configuration: Add to application.yml or create application-local.yml:

spring:
  sql:
    init:
      mode: always  # or 'embedded' for H2 only
      platform: local

Impact: The sample data file won't be loaded automatically, making it appear non-functional to developers.

Recommendation: Either:

  • Add the configuration to enable SQL initialization
  • Document manual loading steps in the PR description
  • Consider using Flyway/Liquibase for repeatable migrations

3. Docker Compose Configuration (compose.yaml:8-13)

Security Observation: While the warning comment is present, consider these improvements for environment variable overrides and add health checks for better reliability.

Additional Considerations:

  • Consider adding restart policy: restart: unless-stopped
  • Consider exposing port only to localhost: 127.0.0.1:3306:3306

4. Sample Data Quality Issues (src/main/resources/data-local.sql)

Issues:

  • Hardcoded dates (2025-2027) will become stale and eventually all past
  • Uses INSERT IGNORE which silently fails - harder to debug
  • ID values are hardcoded, may conflict with auto-increment sequences

Recommendations:

  • Consider using dynamic dates with DATE_ADD/INTERVAL for MySQL
  • Or use a dedicated test data generator in Java with @Profile("local")

📋 Test Coverage

Current Status: ✅ All 116 tests passing

Missing Test Coverage:

  1. No tests verify the removal of @FutureOrPresent validation
  2. No tests for Docker Compose integration
  3. Consider adding integration test for data-local.sql loading

Recommendation: Add a test case to verify the new validation behavior with past dates.


🎯 Action Items

Before Merge:

  1. ❗ Document why @FutureOrPresent was removed or restore it with appropriate handling
  2. ❗ Add SQL initialization configuration for data-local.sql
  3. ⚠️ Consider using dynamic dates in SQL or Java-based data loader

Nice to Have:

  • Add health checks to Docker Compose
  • Add test for past date validation behavior
  • Document the Docker Compose setup in README or CLAUDE.md

Overall Assessment

Code Quality: ✅ Good
Security: ✅ Adequate (with warnings in place)
Testing: ⚠️ Missing validation tests
Documentation: ⚠️ Needs SQL init config documentation

Recommendation: Request changes to address the critical validation removal and SQL initialization configuration before merging.


Review generated by Claude Code following Spring User Framework conventions

@devondragon devondragon deleted the feature/spring-user-framework-4.0.2-snapshot branch February 22, 2026 01:17
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