fix: fail to delete multiple files if the files are removed from upstream storage #2317

JackyLiang522 · 2022-08-05T09:22:48Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

修复了在云端删除文件后，在Halo同时删除多个附件失败的bug。

Halo数据库里的附件并不会和云端文件实时同步，如果在云端删除文件多个后又在Halo删除，在删除第一个文件后则会抛出异常导致其它文件无法在halo的数据库中正常删除。这个PR通过删除在删除又拍云和七牛云的附件后判断返回状态和抛出异常的代码来解决这个问题。

Which issue(s) this PR fixes:

Fixes #2283

Special notes for your reviewer:

该bug只在七牛云和又拍云存在，其它云储存没有判断返回状态和抛出异常的代码，因此不存在这个问题。

Does this PR introduce a user-facing change?

NONE

CLAassistant · 2022-08-05T09:22:54Z

All committers have signed the CLA.

src/main/java/run/halo/app/handler/file/QiniuOssFileHandler.java

src/main/java/run/halo/app/handler/file/UpOssFileHandler.java

ruibaby · 2022-08-05T13:11:32Z

/cc @halo-dev/sig-halo

guqing · 2022-08-09T07:51:02Z

src/main/java/run/halo/app/handler/file/QiniuOssFileHandler.java

@@ -186,7 +186,6 @@ public void delete(String key) {
            Response response = bucketManager.delete(bucket, key);
            if (!response.isOK()) {
                log.warn("附件 " + key + " 从七牛云删除失败");
-                throw new FileOperationException("附件 " + key + " 从七牛云删除失败");


如果删除了异常catch，那么可能因为网络问题而导致删除失败时，halo中的数据库记录却被删除了的问题从而导致文件遗留。
建议先查询一次来判断 bucket 中是否存在文件，如果不存在则直接跳到删除数据库记录的逻辑，否则不需要改变原来的逻辑，用户可以通过手动重试来解决

在原来的代码里，删除附件时会先在数据库里删除后向云端发起删除请求，所以就算抛出了异常还是会有文件遗失问题。如果想要通过抛出异常防止文件遗失，可能要先将这个顺序改过来？

在原来的代码里，删除附件时会先在数据库里删除后向云端发起删除请求，所以就算抛出了异常还是会有文件遗失问题。如果想要通过抛出异常防止文件遗失，可能要先将这个顺序改过来？

那对于这个方法可以在进入时查询一下oss bucket中该文件是否存在，不存在直接return即可，存在再删除，这样就不需要去改原来的顺序问题了，因为issue的问题就是通过其他途径删除了bucket中的该文件导致抛出异常，如果文件不存在直接return即只有数据库被删除是合理的

JohnNiang · 2022-08-09T10:14:12Z

建议根据 OSS SDK 检查一下文件已删除的错误，再决定是否执行 delete 操作。

JackyLiang522 · 2022-08-09T11:08:13Z

建议根据 OSS SDK 检查一下文件已删除的错误，再决定是否继续抛出异常。

通过判断请求返回的status_code是否为404可以知道文件是否已在云端删除，是否需要添加这一层判断？

JackyLiang522 · 2022-08-09T11:26:32Z

我重新修改了一下代码，目前的逻辑是附件正常删除或者附件已经在云端被删除（即请求返回的status_code为404）时不抛出异常，其它情况下抛出异常，这样处理是否可行？

JohnNiang · 2022-08-15T15:22:36Z

/cc @halo-dev/sig-halo

src/main/java/run/halo/app/handler/file/QiniuOssFileHandler.java

src/main/java/run/halo/app/handler/file/UpOssFileHandler.java

JohnNiang

Thank you for your first contribution!
/approve

ruibaby

/lgtm

Thanks for your first contribution.

ruibaby · 2022-08-18T06:54:49Z

/milestone 1.5.x

ruibaby · 2022-08-18T06:54:58Z

/cherry-pick release-1.5

halo-dev-bot · 2022-08-18T06:55:01Z

@ruibaby: once the present PR merges, I will cherry-pick it on top of release-1.5 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ruibaby · 2022-08-18T06:56:21Z

@JustinLiang522 还有一点 CheckStyle 的问题，导致 Merge 被 Block。你可以按照文档配置一下：https://docs.halo.run/developer-guide/core/code-style

https://github.com/halo-dev/halo/pull/2317/checks

ruibaby

/lgtm

f2c-ci-robot · 2022-08-18T08:05:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JohnNiang, ruibaby

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [JohnNiang,ruibaby]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

halo-dev-bot · 2022-08-18T08:06:54Z

@ruibaby: new pull request created: #2332

In response to this:

/cherry-pick release-1.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

…oved from upstream storage (#2332) This is an automated cherry-pick of #2317 /assign ruibaby ```release-note NONE ```

@ntdgy

#### What type of PR is this? #### What this PR does / why we need it: 发布 1.6.0 #### Special notes for your reviewer: /cc @halo-dev/sig-halo ```markdown ## Features - SMTP 添加 STARTTLS 的设置支持。 #1861 halo-dev/console#552 @ntdgy @wangxiaoerYah - 为邮件通知模板添加更多的可用参数，`email`、`status`、`createTime`、`authorUrl`。 #2095 @Yhcrown @iRoZhi - 后台附件设置中添加 `去除图片 EXIF 信息` 的选项。 #2168 halo-dev/console#554 @SladeGranger @eziosudo @52lemon6 - 后台文章标签管理支持清理未使用标签。 halo-dev/console#587 @ruibaby - 重构后台的友情链接管理页面，现在支持拖动排序以及分组管理。 halo-dev/console#574 #2105 @Camsyn @daifiyum @gungnir479 ## Improvements - 优化文章编辑时间更新的逻辑，目前更改为仅在修改文章标题或者内容时才更新。 #2195 @wxyShine @ListenV - 修改后台管理页面为直接渲染而不是重定向，以更好地兼容反向代理。 #2259 @viticis - 后台在构建时提供 gzip 压缩的资源。 halo-dev/console#547 @2211898719 - 后台主题设置界面顶部提供了保存设置的按钮。 halo-dev/console#549 @Aanko - 修改后台页面的 lang 属性，由 `zh-cmn-Hans` 改为 `cmn-Han`，`zh-cmn-Hans` 已废弃。 halo-dev/console#576 @wordlesswind - 优化后台附件库列表以及选择附件弹框的布局。 halo-dev/console#580 @ruibaby - 修改后台底部的 `Power by Halo` 为 `Powered by Halo`。 halo-dev/console#597 @liaocp666 - 优化后台日志管理设置公开/隐藏状态的方式。 halo-dev/console#610 @zjy4fun @manction - 优化后台在个人资料中设置头像的逻辑，现在无需再提交个人资料表单。 halo-dev/console#619 @wxyShine - 优化后台图库批量从附件添加时，按照选择顺序倒序排列，即最先选择的图片在最前面。 halo-dev/console#631 @zjy4fun @zyy247796143 ## Bug Fixes - 修复文章详情页 `meta_description` 为空的问题。 #2282 @guqing @ruibaby - 修复批量删除外部云存储的时候，因为文件不存在导致的删除失败问题。 #2317 @JustinLiang522 @129duckflew - 修复了七牛云存储附件无法上传非图片文件的问题。 #2331 @AirboZH @hexWars - 修复未审核评论回复会发送邮件提醒的问题。 #2340 @AirboZH @cetr - 修复在单个分类所属文章页面页面变量中无法获取子分类中文章的问题。 #2405 @JustinLiang522 @HugeLeaf - 修复管理员资料表单中邮件地址的字符数限制。 halo-dev/console#571 @Yhcrown - 修复了无法设置日志、相册、链接页标题和每页显示条数的问题。 halo-dev/console#601 @JustinLiang522 @manction ## Dependencies - Docker 镜像的基础镜像使用 Eclipse Temurin 镜像替代 AdoptOpenJDK。 #2120 @wordlesswind ``` #### Does this PR introduce a user-facing change? ```release-note None ```

@cetr

#### What type of PR is this? /milestone 1.5.x #### What this PR does / why we need it: 发布 1.5.5 #### Special notes for your reviewer: ```markdown ## Improvements - 完善install路径返回的页面结果。#2225 @cetr ## Bug Fixes - 修改根据分类查询文章方法，使其同时返回子分类文章。#2405 @JustinLiang522 - 修复七牛云做附件存储时附件无法上传非图片文件的问题。#2331 @AirboZH - 修复了当云存储中文件被删除时，在 Halo 同时删除多个附件会失败的问题。#2317 @JustinLiang522 - 修复文章详情页 meta_description 内容生成为空的问题。#2282 @guqing - 修复了无法设置日志、相册、链接页标题和每页显示条数的问题。halo-dev/console#601 @JustinLiang522 ``` /cc @halo-dev/sig-halo #### Does this PR introduce a user-facing change? ```release-note None ```

fix issue 2283

3f5a4a1

f2c-ci-robot bot added release-note-none Denotes a PR that doesn't merit a release note. kind/bug Categorizes issue or PR as related to a bug. labels Aug 5, 2022

f2c-ci-robot bot requested review from lan-yonghui and wan92hen August 5, 2022 09:22

JackyLiang522 changed the title ~~Fixes #2283~~ fix: fail to delete multiple files if the files are first removed from cloud Aug 5, 2022

ruibaby requested changes Aug 5, 2022

View reviewed changes

src/main/java/run/halo/app/handler/file/QiniuOssFileHandler.java Outdated Show resolved Hide resolved

src/main/java/run/halo/app/handler/file/UpOssFileHandler.java Show resolved Hide resolved

reserve logs

390b244

JackyLiang522 requested a review from ruibaby August 5, 2022 13:42

ruibaby requested review from guqing and JohnNiang August 9, 2022 07:43

guqing reviewed Aug 9, 2022

View reviewed changes

check status

36c40f8

JohnNiang requested changes Aug 17, 2022

View reviewed changes

src/main/java/run/halo/app/handler/file/QiniuOssFileHandler.java Outdated Show resolved Hide resolved

JohnNiang reviewed Aug 17, 2022

View reviewed changes

src/main/java/run/halo/app/handler/file/UpOssFileHandler.java Outdated Show resolved Hide resolved

JackyLiang522 added 2 commits August 17, 2022 14:36

change qiniu code

63be71a

update upyun code

7b0299e

JohnNiang reviewed Aug 17, 2022

View reviewed changes

src/main/java/run/halo/app/handler/file/UpOssFileHandler.java Outdated Show resolved Hide resolved

use jsonUtil

b6f245a

JohnNiang approved these changes Aug 18, 2022

View reviewed changes

f2c-ci-robot bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 18, 2022

ruibaby approved these changes Aug 18, 2022

View reviewed changes

f2c-ci-robot bot assigned ruibaby Aug 18, 2022

f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Aug 18, 2022

f2c-ci-robot bot added this to the 1.5.x milestone Aug 18, 2022

ruibaby changed the title ~~fix: fail to delete multiple files if the files are first removed from cloud~~ fix: fail to delete multiple files if the files are removed from upstream storage Aug 18, 2022

checkstyle

9e861a2

f2c-ci-robot bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 18, 2022

ruibaby approved these changes Aug 18, 2022

View reviewed changes

f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Aug 18, 2022

f2c-ci-robot bot merged commit 13268cb into halo-dev:master Aug 18, 2022

halo-dev-bot mentioned this pull request Aug 18, 2022

[release-1.5] fix: fail to delete multiple files if the files are removed from upstream storage #2332

Merged

f2c-ci-robot bot pushed a commit that referenced this pull request Aug 18, 2022

[release-1.5] fix: fail to delete multiple files if the files are rem…

b2925e4

…oved from upstream storage (#2332) This is an automated cherry-pick of #2317 /assign ruibaby ```release-note NONE ```

ruibaby mentioned this pull request Aug 21, 2022

对象存储内的文件在控制台删除后，halo后台已在腾讯云删除附件不会同步删除，且无法手动删除 #2338

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: fail to delete multiple files if the files are removed from upstream storage #2317

fix: fail to delete multiple files if the files are removed from upstream storage #2317

JackyLiang522 commented Aug 5, 2022

CLAassistant commented Aug 5, 2022 •

edited

Loading

ruibaby commented Aug 5, 2022

guqing Aug 9, 2022

JackyLiang522 Aug 9, 2022

guqing Aug 9, 2022 •

edited

Loading

JohnNiang commented Aug 9, 2022 •

edited

Loading

JackyLiang522 commented Aug 9, 2022

JackyLiang522 commented Aug 9, 2022

JohnNiang commented Aug 15, 2022

JohnNiang left a comment

ruibaby left a comment

ruibaby commented Aug 18, 2022

ruibaby commented Aug 18, 2022

halo-dev-bot commented Aug 18, 2022

ruibaby commented Aug 18, 2022

ruibaby left a comment

f2c-ci-robot bot commented Aug 18, 2022

halo-dev-bot commented Aug 18, 2022

fix: fail to delete multiple files if the files are removed from upstream storage #2317

fix: fail to delete multiple files if the files are removed from upstream storage #2317

Conversation

JackyLiang522 commented Aug 5, 2022

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

CLAassistant commented Aug 5, 2022 • edited Loading

ruibaby commented Aug 5, 2022

guqing Aug 9, 2022

Choose a reason for hiding this comment

JackyLiang522 Aug 9, 2022

Choose a reason for hiding this comment

guqing Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

JohnNiang commented Aug 9, 2022 • edited Loading

JackyLiang522 commented Aug 9, 2022

JackyLiang522 commented Aug 9, 2022

JohnNiang commented Aug 15, 2022

JohnNiang left a comment

Choose a reason for hiding this comment

ruibaby left a comment

Choose a reason for hiding this comment

ruibaby commented Aug 18, 2022

ruibaby commented Aug 18, 2022

halo-dev-bot commented Aug 18, 2022

ruibaby commented Aug 18, 2022

ruibaby left a comment

Choose a reason for hiding this comment

f2c-ci-robot bot commented Aug 18, 2022

halo-dev-bot commented Aug 18, 2022

CLAassistant commented Aug 5, 2022 •

edited

Loading

guqing Aug 9, 2022 •

edited

Loading

JohnNiang commented Aug 9, 2022 •

edited

Loading