Skip to content

Refactor/domain foundation | 实施计划中的 Phase 1 基座改造与版本化迁移#19

Open
Self4215 wants to merge 3 commits intomasterfrom
refactor/domain-foundation
Open

Refactor/domain foundation | 实施计划中的 Phase 1 基座改造与版本化迁移#19
Self4215 wants to merge 3 commits intomasterfrom
refactor/domain-foundation

Conversation

@Self4215
Copy link
Copy Markdown
Owner

Phase 1 基座改造与版本化迁移

本阶段在不改变 Spring Boot + Thymeleaf 主体架构的前提下,优先完成可持续演进底座:统一角色语义、稳定领域模型、引入可回滚迁移、预埋采集与导入导出扩展点、统一异常与审计口径。这样可避免 Phase 2-7 重复改表和改权限,保证后续功能可并行推进且回归成本可控。

以下是 Copilot 为本 PR 生成的 Summary:


本次 PR 引入了项目“Phase 1 领域基础”的初始实现,重点是为未来开发建立一个稳健且可扩展的基础。主要变更包括详细的迁移与重构计划、安全与错误处理层的改进、日志与审计的增强,以及新领域模型和数据库迁移工具的引入。

领域基础与迁移

  • .github/prompts/phase1-domain-foundation.plan.md 中添加了 Phase 1 领域基础与迁移的全面计划,详细说明了角色统一、领域模型稳定、迁移策略、错误处理和可扩展性要点等步骤。
  • 将项目版本升级至 2.1.0,并在 pom.xml 中添加了 Flyway 依赖,以支持可版本化、可审计且支持回滚的数据库迁移。[1] [2]

安全与访问控制

  • 增强了 SecurityConfig.java,以实现:
    • 允许匿名浏览商品,并对 /sales/**/admin/**/cart/**/order/** 端点实施细粒度访问控制。
    • 添加自定义的 AccessDeniedHandler,以统一处理 403 错误。
    • 集成静态资源和错误页面的访问规则。
  • 引入了新的 SalesController/sales/dashboard 路由,并支持身份验证后基于角色的重定向。[1] [2]

错误处理与审计

  • GlobalControllerAdvice.java 中实现了全局异常处理,为业务、资源和系统错误提供了具有一致元数据的标准化错误响应。
  • 添加了专用的 ErrorPageController 和一个通用的错误页面端点,以用户友好的格式显示错误详情。

日志与审计增强

  • 扩展了登录、登出和商品浏览操作,以记录详细的审计日志,包括操作类型、模块、结果、客户端 IP 和会话 ID。[1] [2] [3]

领域模型与服务更新

  • 引入了新的 Category 实体以支持商品分类,包含代码、名称、描述、启用状态和时间戳等字段。
  • 更新了商品列表功能,使用 getActiveProducts() 并支持匿名浏览,同时改进了日志记录和用户界面上下文。

这些变更为一个稳定、可扩展且可审计的系统奠定了基础,最大限度地降低了未来的迁移风险,并支持后续阶段的并行开发。

- 重构用户角色,采用标准化命名(CUSTOMER、SALES、ADMIN)。
- 引入 `Category` 实体和仓储,用于商品分类管理。
- 增强 `Product` 实体,增加库存和状态字段。
- 扩展 `OperationLog` 实体,包含操作者角色、模块、操作类型、目标类型、结果、IP 地址和追踪 ID。
- 更新安全配置,使访问控制基于用户角色。
- 修改认证和退出处理器,记录带详细信息的操作日志。
- 为已认证用户实现带日志记录的商品列表功能。
- 创建销售仪表板控制器和视图,为未来的销售功能做准备。
- 使用 Flyway 将现有数据迁移到新架构,以便版本控制。
- 更新应用程序属性,配置数据库初始化和 Flyway。
- 调整 `data.sql` ,初始化数据并确保与新架构兼容。
- 实现自定义 `AccessDeniedHandler` ,用于管理 403 错误并补充详细属性。
- 添加 `ErrorPageController` ,用于处理通用错误响应并展示更友好的提示信息。
- 引入 `AppException` 和 `AppErrorCode` ,为各个服务提供结构化错误处理。
- 调整 `OrderService` 和 `ProductService` ,在业务逻辑出错时抛出 `AppException` 。
- 修改应用配置,启用 Flyway 进行数据库迁移。
- 创建迁移脚本,确保数据库结构更新是安全且幂等的。
- 添加 `OrderService` 和 `ProductService` 的单元测试,用于验证错误处理和业务逻辑。
- 添加安全访问集成测试,确保授权与错误处理行为正确。
从现在开始,正式修改版本号规范。
我知道版本号一般应从 0 开始,但为了能跟 plan 中的
Phase X (1, 2, ...) 对应上,就从 1 开始吧。
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Phase 1 “domain foundation” refactor that normalizes role semantics, introduces versioned DB migrations, adds a Sales dashboard entrypoint, and unifies error/audit behavior while keeping the Spring Boot + Thymeleaf architecture intact.

Changes:

  • Introduce Flyway migrations + seed data; expand domain model (Category, Product stock/status, extended operation logs) and role normalization.
  • Add unified error model (AppException/AppErrorCode) + global error handling and an error page.
  • Refine security/routing (anonymous product browsing, Sales/Admin separation) and add baseline tests for services and access control.

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/test/java/org/self4215/web/SecurityAccessIntegrationTest.java Web MVC security access tests for anonymous/role-based routes and 403 handling
src/test/java/org/self4215/service/ProductServiceTest.java Unit tests for product defaults and not-found error code
src/test/java/org/self4215/service/OrderServiceTest.java Unit tests for order creation, totals, and business-conflict handling
src/test/java/org/self4215/config/GlobalControllerAdviceTest.java Unit tests for error handler footer metadata and 404 mapping
src/main/resources/templates/sales/dashboard.html New Sales dashboard placeholder page
src/main/resources/templates/products.html UI changes for anonymous browsing vs authenticated actions
src/main/resources/templates/error/common.html New unified error page template
src/main/resources/templates/admin/products.html Admin product list shows stock/status
src/main/resources/templates/admin/product_form.html Admin product form adds stock/status fields
src/main/resources/db/migration/V2__seed_base_data.sql Seed + compatibility migration for baselined environments
src/main/resources/db/migration/V1__phase1_domain_foundation.sql Phase 1 schema evolution (roles, category/stock, log expansion, indexes)
src/main/resources/data.sql Updated legacy SQL init seed/normalization (now overlaps with Flyway path)
src/main/resources/application.properties Enable/configure Flyway; disable SQL init; keep JPA update as transitional strategy
src/main/java/org/self4215/service/UserService.java Role normalization + AppException usage
src/main/java/org/self4215/service/ProductService.java Active-products query + AppException + defaulting stock/status
src/main/java/org/self4215/service/OrderService.java Replace RuntimeExceptions with AppException
src/main/java/org/self4215/service/LogService.java Extend audit log payload and add overload for structured fields
src/main/java/org/self4215/repository/ProductRepository.java Add status-filtered finder
src/main/java/org/self4215/repository/CategoryRepository.java New repository for Category lookup
src/main/java/org/self4215/exception/AppException.java New application exception type with error code + user message
src/main/java/org/self4215/exception/AppErrorCode.java Centralized error code enum
src/main/java/org/self4215/entity/UserRole.java New canonical role enum
src/main/java/org/self4215/entity/User.java Add province/city/updatedAt and adjust role semantics
src/main/java/org/self4215/entity/Product.java Add category association + stock/status fields
src/main/java/org/self4215/entity/OperationLog.java Expand structured audit log fields
src/main/java/org/self4215/entity/Category.java New Category entity with timestamps
src/main/java/org/self4215/controller/SalesController.java New Sales dashboard controller
src/main/java/org/self4215/controller/ProductController.java Anonymous browsing support + structured browse logging
src/main/java/org/self4215/controller/OrderController.java Remove controller-level try/catch to rely on global error handling
src/main/java/org/self4215/controller/ErrorPageController.java Controller endpoint to render a common error page from request attributes
src/main/java/org/self4215/config/SecurityConfig.java Fine-grained authorization + unified 403 handler forwarding to error page
src/main/java/org/self4215/config/GlobalControllerAdvice.java Global exception handlers mapping AppErrorCode -> HTTP status + shared footer metadata
src/main/java/org/self4215/config/CustomLogoutSuccessHandler.java Structured logout audit logging with IP/session info
src/main/java/org/self4215/config/CustomAuthenticationSuccessHandler.java Structured login audit logging + Sales role redirect
pom.xml Version bump + add Flyway and security-test deps
.github/prompts/phase1-domain-foundation.plan.md Phase 1 implementation plan and scope boundaries

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


private String imageUrl; // 商品图片(用占位图)

@ManyToOne
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

@ManyToOne defaults to FetchType.EAGER, which can introduce N+1 queries (e.g., loading the product list will also fetch each product’s category even if the view doesn’t need it). Set this association to fetch = FetchType.LAZY and fetch-join explicitly where category data is needed.

Suggested change
@ManyToOne
@ManyToOne(fetch = FetchType.LAZY)

Copilot uses AI. Check for mistakes.
Comment on lines +65 to +68
String forwardedFor = request.getHeader("X-Forwarded-For");
if (forwardedFor != null && !forwardedFor.isBlank()) {
return forwardedFor.split(",")[0].trim();
}
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

extractClientIp is duplicated across multiple classes and trusts the X-Forwarded-For header unconditionally, which makes audit IPs trivially spoofable unless the app is behind a trusted proxy with forwarded-header handling configured. Consider centralizing this logic (e.g., a shared util/service) and using Spring’s forwarded header support (ForwardedHeaderFilter / server.forward-headers-strategy) so you only trust proxy headers when appropriate.

Suggested change
String forwardedFor = request.getHeader("X-Forwarded-For");
if (forwardedFor != null && !forwardedFor.isBlank()) {
return forwardedFor.split(",")[0].trim();
}
// Do not trust X-Forwarded-For directly here. If the application is deployed
// behind a trusted proxy, Spring/container forwarded-header support should be
// configured so request.getRemoteAddr() already reflects the real client IP.

Copilot uses AI. Check for mistakes.
Comment on lines +60 to +63
String forwardedFor = request.getHeader("X-Forwarded-For");
if (forwardedFor != null && !forwardedFor.isBlank()) {
return forwardedFor.split(",")[0].trim();
}
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

extractClientIp is duplicated across multiple classes and trusts the X-Forwarded-For header unconditionally, which makes audit IPs trivially spoofable unless the app is behind a trusted proxy with forwarded-header handling configured. Consider centralizing this logic (e.g., a shared util/service) and using Spring’s forwarded header support (ForwardedHeaderFilter / server.forward-headers-strategy) so you only trust proxy headers when appropriate.

Suggested change
String forwardedFor = request.getHeader("X-Forwarded-For");
if (forwardedFor != null && !forwardedFor.isBlank()) {
return forwardedFor.split(",")[0].trim();
}

Copilot uses AI. Check for mistakes.
Comment on lines +41 to +53
extractClientIp(request),
request.getRequestedSessionId());
}
response.sendRedirect("/login?logout");
}

private String extractClientIp(HttpServletRequest request) {
String forwardedFor = request.getHeader("X-Forwarded-For");
if (forwardedFor != null && !forwardedFor.isBlank()) {
return forwardedFor.split(",")[0].trim();
}
return request.getRemoteAddr();
}
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

extractClientIp is duplicated across multiple classes and trusts the X-Forwarded-For header unconditionally, which makes audit IPs trivially spoofable unless the app is behind a trusted proxy with forwarded-header handling configured. Consider centralizing this logic (e.g., a shared util/service) and using Spring’s forwarded header support (ForwardedHeaderFilter / server.forward-headers-strategy) so you only trust proxy headers when appropriate.

Suggested change
extractClientIp(request),
request.getRequestedSessionId());
}
response.sendRedirect("/login?logout");
}
private String extractClientIp(HttpServletRequest request) {
String forwardedFor = request.getHeader("X-Forwarded-For");
if (forwardedFor != null && !forwardedFor.isBlank()) {
return forwardedFor.split(",")[0].trim();
}
return request.getRemoteAddr();
}
request.getRemoteAddr(),
request.getRequestedSessionId());
}
response.sendRedirect("/login?logout");
}

Copilot uses AI. Check for mistakes.
Comment on lines +31 to +35
-- 兼容历史数据:标准化角色与商品状态,避免老数据无法通过新逻辑展示
UPDATE users SET role = 'CUSTOMER' WHERE role = 'ROLE_CUSTOMER';
UPDATE users SET role = 'ADMIN' WHERE role = 'ROLE_ADMIN';
UPDATE users SET role = 'SALES' WHERE role = 'ROLE_SALES';
UPDATE users SET role = 'CUSTOMER' WHERE role IS NULL OR role = '';
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

data.sql is still being maintained, but spring.sql.init.mode=never disables SQL init in the default config, so this seed/normalization logic won’t run. To avoid confusion/drift, consider removing data.sql, migrating all seed data into Flyway, or scoping data.sql to a dedicated Spring profile where SQL init is enabled.

Copilot uses AI. Check for mistakes.
Comment on lines +16 to +27
SET @sql := (
SELECT IF(
EXISTS(
SELECT 1
FROM information_schema.columns
WHERE table_schema = DATABASE()
AND table_name = 'users'
AND column_name = 'province'
),
'SELECT 1',
'ALTER TABLE users ADD COLUMN province VARCHAR(255)'
)
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

Flyway V1 migration assumes existing tables (e.g., users) and will execute ALTER TABLE users ... when the table is missing. On a fresh database, Flyway runs before Hibernate ddl-auto=update, so this migration will fail immediately. To support new deployments, add baseline schema creation migrations (create users/products/operation_logs/... tables) or make V1 conditional on table existence and ensure schema is created elsewhere before Flyway runs.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +5
-- Phase 1 seed data moved from data.sql to Flyway.
-- V2 需兼容两种状态:
-- 1) 新库按 V1 -> V2 执行
-- 2) 旧库 baseline=1,V1 跳过后直接执行 V2

Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

V2 is intended to run when existing databases are baselined to version 1 (skipping V1), but it currently omits the index creation steps present in V1 (e.g., idx_products_category_id, idx_operation_logs_module). That means baselined environments won’t get these indexes at all. Consider adding the same idempotent index-creation logic to V2 (or moving indexes into a separate migration that always runs).

Copilot uses AI. Check for mistakes.
Comment on lines +249 to +257
INSERT INTO categories (id, code, name, description, enabled)
VALUES
(1, 'PHONE', '手机数码', '手机与配件', 1),
(2, 'COMPUTER', '电脑办公', '电脑与办公设备', 1),
(3, 'AUDIO', '音频设备', '耳机音响等设备', 1)
ON DUPLICATE KEY UPDATE
name = VALUES(name),
description = VALUES(description),
enabled = VALUES(enabled);
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

Seeding categories with hard-coded primary keys (1,2,3) can break or silently misbehave on databases where those IDs are already occupied (the ON DUPLICATE KEY path will update the existing row by PK, but won’t guarantee the expected code rows exist). Prefer inserting without id and relying on the unique code key for upserts, or ensure IDs cannot conflict.

Copilot uses AI. Check for mistakes.
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