fix: modify resource upload #264
Conversation
|
Warning Rate limit exceeded@lu-yg has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughRefactors resource retrieval from base64 data to name-based lookup with an isResource flag. Updates controller endpoint signature, service interface, and implementation accordingly. Removes category from ResourceRequestDto. Upload logic now assigns unique names and builds download/thumbnail URLs; duplicate handling via ServiceException. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Controller as ResourceController
participant Service as ResourceService
participant Repo as ResourceRepository
rect rgba(230,245,255,0.5)
note over Client,Controller: Download by name
Client->>Controller: GET /resource?name=...&isResource=bool
Controller->>Controller: URL decode(name)
Controller->>Service: queryResourceByName(name)
Service->>Repo: findByName(name)
Repo-->>Service: Resource or null
alt Resource found
Service-->>Controller: Resource
Controller->>Controller: Select data (resource vs thumbnail)
Controller-->>Client: 200 OK + bytes (Content-Disposition)
else Not found
Controller-->>Client: ServiceException (CM009)
end
end
sequenceDiagram
autonumber
actor Client
participant Service as ResourceServiceImpl
participant Repo as ResourceRepository
rect rgba(235,255,235,0.5)
note over Client,Service: Upload flow (name/URL-based)
Client->>Service: resourceUpload(Resource)
Service->>Service: Generate unique name (Instant-based)
Service->>Service: Build URLs with ?name=...&isResource=true/false
Service->>Repo: Check duplicate by name
alt Duplicate
Service-->>Client: throw ServiceException
else New
Service->>Repo: save(Resource)
Repo-->>Service: Persisted Resource
Service-->>Client: Persisted Resource with URLs
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java (1)
189-196: Also enforce app/tenant in duplicate check.Currently checking only
nameandcategorycan cross apps.Apply:
QueryWrapper<Resource> queryWrapper = new QueryWrapper<>(); queryWrapper.eq("name", resource.getName()); queryWrapper.eq("category", resource.getCategory()); + queryWrapper.eq("app_id", loginUserContext.getAppId());
🧹 Nitpick comments (4)
base/src/main/java/com/tinyengine/it/service/material/ResourceService.java (1)
47-52: Drop unnecessary checked exception from the new API.
queryResourceByName(String name)doesn’t throw checked exceptions in the impl; keep the interface clean by removingthrows Exceptionfor this method.Apply:
- Resource queryResourceByName(String name) throws Exception; + Resource queryResourceByName(String name);base/src/main/java/com/tinyengine/it/controller/ResourceController.java (1)
312-312: Narrow the throws clause.
getOutputStream()only throwsIOException. Preferthrows IOException(or let your global exception handler translate it).base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java (2)
175-178: Reduce collision risk in generated names.Concatenating epoch millis with the original name can collide under concurrent uploads. Add a delimiter and a UUID.
Apply:
- String imageName = Instant.now().toEpochMilli()+resource.getName(); + String imageName = Instant.now().toEpochMilli() + "-" + java.util.UUID.randomUUID() + "-" + resource.getName();
181-188: Guard against empty resource data.Persisting without
resourceUrl/thumbnailUrlif data is empty leads to unusable records.Apply:
- if (!StringUtils.isEmpty(resourceData)) { + if (!StringUtils.isEmpty(resourceData)) { ... - } + } else { + throw new ServiceException(ExceptionEnum.CM002.getResultCode(), "Resource data is empty"); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
base/src/main/java/com/tinyengine/it/controller/ResourceController.java(3 hunks)base/src/main/java/com/tinyengine/it/model/dto/ResourceRequestDto.java(0 hunks)base/src/main/java/com/tinyengine/it/service/material/ResourceService.java(1 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java(3 hunks)
💤 Files with no reviewable changes (1)
- base/src/main/java/com/tinyengine/it/model/dto/ResourceRequestDto.java
🧰 Additional context used
🧬 Code graph analysis (1)
base/src/main/java/com/tinyengine/it/controller/ResourceController.java (1)
base/src/main/java/com/tinyengine/it/common/utils/ImageThumbnailGenerator.java (1)
ImageThumbnailGenerator(42-515)
🔇 Additional comments (1)
base/src/main/java/com/tinyengine/it/controller/ResourceController.java (1)
324-324: LGTM: filename logic.
thumbnail_prefix selection for derived files is correct and user-friendly.
base/src/main/java/com/tinyengine/it/controller/ResourceController.java
Outdated
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/controller/ResourceController.java
Outdated
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java
Show resolved
Hide resolved
* fix: modify resource upload
3 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores