Refactor/domain foundation | 实施计划中的 Phase 1 基座改造与版本化迁移#19
Refactor/domain foundation | 实施计划中的 Phase 1 基座改造与版本化迁移#19
Conversation
- 重构用户角色,采用标准化命名(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 开始吧。
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
@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.
| @ManyToOne | |
| @ManyToOne(fetch = FetchType.LAZY) |
| String forwardedFor = request.getHeader("X-Forwarded-For"); | ||
| if (forwardedFor != null && !forwardedFor.isBlank()) { | ||
| return forwardedFor.split(",")[0].trim(); | ||
| } |
There was a problem hiding this comment.
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.
| 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. |
| String forwardedFor = request.getHeader("X-Forwarded-For"); | ||
| if (forwardedFor != null && !forwardedFor.isBlank()) { | ||
| return forwardedFor.split(",")[0].trim(); | ||
| } |
There was a problem hiding this comment.
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.
| String forwardedFor = request.getHeader("X-Forwarded-For"); | |
| if (forwardedFor != null && !forwardedFor.isBlank()) { | |
| return forwardedFor.split(",")[0].trim(); | |
| } |
| 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(); | ||
| } |
There was a problem hiding this comment.
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.
| 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"); | |
| } |
| -- 兼容历史数据:标准化角色与商品状态,避免老数据无法通过新逻辑展示 | ||
| 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 = ''; |
There was a problem hiding this comment.
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.
| 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)' | ||
| ) |
There was a problem hiding this comment.
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.
| -- Phase 1 seed data moved from data.sql to Flyway. | ||
| -- V2 需兼容两种状态: | ||
| -- 1) 新库按 V1 -> V2 执行 | ||
| -- 2) 旧库 baseline=1,V1 跳过后直接执行 V2 | ||
|
|
There was a problem hiding this comment.
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).
| 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); |
There was a problem hiding this comment.
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.
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和一个通用的错误页面端点,以用户友好的格式显示错误详情。日志与审计增强
领域模型与服务更新
Category实体以支持商品分类,包含代码、名称、描述、启用状态和时间戳等字段。getActiveProducts()并支持匿名浏览,同时改进了日志记录和用户界面上下文。这些变更为一个稳定、可扩展且可审计的系统奠定了基础,最大限度地降低了未来的迁移风险,并支持后续阶段的并行开发。