Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add upload link and download link API #683

Merged
merged 10 commits into from
Sep 3, 2024
Merged

Add upload link and download link API #683

merged 10 commits into from
Sep 3, 2024

Conversation

feiniks
Copy link
Contributor

@feiniks feiniks commented Aug 16, 2024

No description provided.

@@ -1170,13 +1169,13 @@ access_zip_cb (evhtp_request_t *req, void *arg)
int error_code;

char **parts = g_strsplit (req->uri->path->full + 1, "/", 0);
if (g_strv_length (parts) != 2) {
if (g_strv_length (parts) != 4) {
Copy link
Member

Choose a reason for hiding this comment

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

这个 access_zip_cb 不能改的吧,改了就不兼容原有的下载 zip 的方式了。新增的 url 的 cb 应该跟现在的独立,内部可以共用一些代码逻辑。

int
access_file_init (evhtp_t *htp)
{
evhtp_set_regex_cb (htp, "^/files/.*", access_cb, NULL);
evhtp_set_regex_cb (htp, "^/blks/.*", access_blks_cb, NULL);
evhtp_set_regex_cb (htp, "^/zip/.*", access_zip_cb, NULL);
evhtp_set_regex_cb (htp, "^/zip/.*", access_zip_link_cb, NULL);
Copy link
Member

Choose a reason for hiding this comment

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

这个原来的 /zip/{token} URL 应该只想原来的 access_zip_cb。这个不要改。

int
access_file_init (evhtp_t *htp)
{
evhtp_set_regex_cb (htp, "^/files/.*", access_cb, NULL);
evhtp_set_regex_cb (htp, "^/blks/.*", access_blks_cb, NULL);
evhtp_set_regex_cb (htp, "^/zip/.*", access_zip_cb, NULL);
evhtp_set_regex_cb (htp, "^/zip/.*", access_zip_link_cb, NULL);
evhtp_set_regex_cb (htp, "^/repos/[\\da-z]{8}-[\\da-z]{4}-[\\da-z]{4}-[\\da-z]{4}-[\\da-z]{12}/zip/.*", access_zip_cb, NULL);
Copy link
Member

Choose a reason for hiding this comment

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

这个属于登录后下载目录的 URL,目前先不用实现,代码先删掉吧。

evhtp_set_regex_cb (htp, "^/zip/.*", access_zip_link_cb, NULL);
evhtp_set_regex_cb (htp, "^/repos/[\\da-z]{8}-[\\da-z]{4}-[\\da-z]{4}-[\\da-z]{4}-[\\da-z]{12}/zip/.*", access_zip_cb, NULL);
evhtp_set_regex_cb (htp, "^/f/.*", access_link_cb, NULL);
evhtp_set_regex_cb (htp, "^/d/.*", access_dir_link_cb, NULL);
Copy link
Member

Choose a reason for hiding this comment

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

目前就先实现这文件外链的 URL 就好了。其中 /d/ 的 URL 先注释掉吧,暂时不用对外提供。

goto out;
}

info = http_tx_manager_query_access_token (token, "dir");
Copy link
Member

Choose a reason for hiding this comment

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

感觉你应该搞错需求了吧。你现在把原来的 /zip/{token} 的 URL 交给了这个 cb 处理。而实际上应该保持原有的 URL 的处理逻辑不变才对。

@@ -10,6 +10,7 @@ typedef struct Progress {
int zipped;
int total;
char *zip_file_path;
char *zip_file_name;
Copy link
Member

Choose a reason for hiding this comment

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

这些和 zip 相关的新增改动都先注释掉吧。

@@ -2641,6 +2647,154 @@ upload_headers_cb (evhtp_request_t *req, evhtp_headers_t *hdr, void *arg)
return EVHTP_RES_OK;
}

static evhtp_res
Copy link
Member

Choose a reason for hiding this comment

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

新的上传外链处理代码也先注释掉吧。

r.Handle("/u/{.*}", appHandler(uploadLinkCB))
r.Handle("/f/{.*}", appHandler(accessLinkCB))
r.Handle("/d/{.*}", appHandler(accessDirLinkCB))
r.Handle("/repos/{repoid:[\\da-z]{8}-[\\da-z]{4}-[\\da-z]{4}-[\\da-z]{4}-[\\da-z]{12}}/zip/{.*}", appHandler(accessZipCB))
Copy link
Member

Choose a reason for hiding this comment

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

go 不分做和 C 部分相同的处理,不用的代码先注释掉。

if info.FilePath == "" {
msg := "Invalid file_path\n"
return &appError{nil, msg, http.StatusBadRequest}
}
Copy link
Member

Choose a reason for hiding this comment

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

内部数据格式有问题应该返回 500。


now := time.Now()
rsp.Header().Set("Last-Modified", now.Format("Mon, 2 Jan 2006 15:04:05 GMT"))
rsp.Header().Set("Cache-Control", "max-age=3600")
Copy link
Member

Choose a reason for hiding this comment

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

Last-Modified 应该设置为文件的最后修改时间吧。

如果请求设置了 If-Modified-Since 头部或者 If-None-Match 头部,那么需要检查文件是否有被更新。参考 https://developer.mozilla.org/en-US/docs/Web/HTTP/Caching#validation 。同时我们应该设置 Cache-Control: no-cache,让客户端先检查缓存是否已经过期。

这些检查和头部设置应该都在获取到文件元信息之后才能做的吧。C 部分也需要相应改动。

if seahubPK == "" {
err := fmt.Errorf("no seahub private key is configured")
return &appError{err, "", http.StatusNotFound}
}
Copy link
Member

Choose a reason for hiding this comment

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

这个改动去掉吧。以后代码提交之后最好自己看一下,有时一些明显不必要的改动就去掉。

fileName := filepath.Base(filePath)
op := "download-link"

if _, ok := r.Header["If-Modified-Since"]; ok {
Copy link
Member

Choose a reason for hiding this comment

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

应该不用处理这个头部吧,现在只要设置了这个头部就返回没变动,也是不对的。

return &appError{nil, msg, http.StatusBadRequest}
}

etag := r.Header.Get("If-None-Match")
Copy link
Member

Choose a reason for hiding this comment

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

这里应该注释一下缓存相关的逻辑是怎样的。包括通过 If-None-Match 头部来检查文件是否有改动,设置 no-cache 来让客户端在使用缓存之前检查文件是否有变化。

}

rsp.Header().Set("ETag", fileID)
rsp.Header().Set("Cache-Control", "no-chche")
Copy link
Member

Choose a reason for hiding this comment

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

no-cache 写错了。这个代码应该没有测试过吧,开发完应该自己先测试一下。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这部分测试了。可能是浏览器使用了默认的Cache-Control。

error_code = EVHTP_RES_OK;
goto out;
}
set_etag (req, file_id);
Copy link
Member

Choose a reason for hiding this comment

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

这里没有设置 no-cache。

}
}

static GList *
Copy link
Member

Choose a reason for hiding this comment

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

下面这两个辅助函数也注释一下吧。

@killing killing merged commit 95bad89 into master Sep 3, 2024
6 checks passed
@killing killing deleted the link_url branch September 3, 2024 10:19
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