Skip to content

feat(file): new file key mechanism #220

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.

Sign up for GitHub

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

Jump to bottom
Merged
weakish merged 5 commits into from
Dec 14, 2020
Merged

feat(file): new file key mechanism #220

weakish merged 5 commits into from
Dec 14, 2020

Conversation

@weakish
Copy link
Contributor

@weakish weakish commented Oct 21, 2020

No description provided.

@weakish
Copy link
Contributor Author

weakish commented Oct 21, 2020 •
edited
Loading

本地跑 File 相关测试均通过：

$ vendor/bin/phpunit test/FileTest.php
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

...........                                                       11 / 11 (100%)

Time: 24.04 seconds, Memory: 4.00MB

OK (11 tests, 30 assertions)

CI 里测试不过是已知问题，和本 pr 无关。等 #217 解决后再 rebase 下本 pr 应该就能都通过了。

Update: 上面的测试指去掉 POST /files/:name 调用的测试。现在已经 rebase 并加了新 commits （SDK 不生成 file key 但允许用户用 master key 指定 key 及相应的测试）。

@weakish weakish force-pushed the deprecate-file-api branch from da464e3 to 1557b63 Compare December 7, 2020 05:47
@weakish weakish changed the title feat(file): update deprecated internal interface feat(file): new file key mechanism Dec 11, 2020
@weakish weakish marked this pull request as ready for review December 11, 2020 11:54
weakish
weakish commented Dec 11, 2020
View reviewed changes
.github/workflows/php.yml
LEANCLOUD_API_SERVER: https://us.avoscloud.com
LEANCLOUD_APP_ID: wnDg0lPt0wcYGJSiHRwHBhD4
LEANCLOUD_APP_KEY: u9ekx9HFSFFBErWwyWHFmPDy
LEANCLOUD_APP_MASTER_KEY: ${{ secrets.MASTER_KEY }}
Copy link
Contributor Author

@weakish weakish Dec 11, 2020

Choose a reason for hiding this comment

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

从设计上说 master key 应该保密，另外也 git grep 了一下之前没有在代码或配置文件中泄露过这个 master key

Copy link
Member

@juvenn juvenn Dec 14, 2020

Choose a reason for hiding this comment

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

是的，之前是在 ci 上面设置的环境变量。

weakish
weakish commented Dec 11, 2020
View reviewed changes
src/LeanCloud/File.php
*/
public function __construct($name, $data=null, $mimeType=null) {
$this->_data["name"] = $name;
$this->_data["key"] = null;
Copy link
Contributor Author

@weakish weakish Dec 11, 2020

Choose a reason for hiding this comment

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

加了一个 key 属性，供用户指定 key

weakish
weakish commented Dec 11, 2020
View reviewed changes
src/LeanCloud/File.php
if ($this->isExternal()) {
$data["url"] = $this->getUrl();
$resp = Client::post("/files/{$this->getName()}", $data);
$resp = Client::post("/files", $data);
Copy link
Contributor Author

@weakish weakish Dec 11, 2020

Choose a reason for hiding this comment

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

api 那边计划废弃 POST /files/:name 接口

weakish
weakish commented Dec 11, 2020
View reviewed changes
src/LeanCloud/File.php
$key = static::genFileKey();
$key = "{$key}." . pathinfo($this->getName(), PATHINFO_EXTENSION);
$data["key"] = $key;
$key = $this->getKey();
Copy link
Contributor Author

@weakish weakish Dec 11, 2020

Choose a reason for hiding this comment

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

SDK 不再生成 key，用户指定了，就传过去，否则不传

weakish
weakish commented Dec 11, 2020
View reviewed changes
test/FileTest.php
$this->assertEmpty($file->getObjectId());
}

public function testSaveExternalFile() {
Copy link
Contributor Author

@weakish weakish Dec 11, 2020

Choose a reason for hiding this comment

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

之前没有保存外部文件的测试，加了一下。

weakish
weakish commented Dec 11, 2020
View reviewed changes
test/MasterTest.php
getenv("LEANCLOUD_APP_ID"),
getenv("LEANCLOUD_APP_KEY"),
getenv("LEANCLOUD_APP_MASTER_KEY"));
Client::useMasterKey(true);
Copy link
Contributor Author

@weakish weakish Dec 11, 2020

Choose a reason for hiding this comment

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

新开一个类，专门测试需要 master 权限的接口。

@weakish
docs(file): typo in doc comment
303df9e
@weakish
test(file): more rigid specified file key test
918e5ca 
- New intl. applications share a gluttony bucket.
- Users may use `/` in the specified key.
weakish
weakish commented Dec 11, 2020
View reviewed changes
Copy link
Contributor Author

@weakish weakish left a comment

Choose a reason for hiding this comment

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

GitHub 现在强制必须留一条总的评论，否则不让提交 inline 评论。这设计明显不合理啊。

weakish
weakish commented Dec 11, 2020
View reviewed changes
Copy link
Contributor Author

@weakish weakish left a comment •
edited
Loading

Choose a reason for hiding this comment

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

改了一下测试（之前没考虑国际版新一代的饕餮应用）。

test/MasterTest.php
$gluttonyPath = "/" + substr(getenv("LEANCLOUD_APP_KEY"), 0, 12) + "/abc";
$this->assertEquals($gluttonyPath, $path);
} else {
$this->assertEquals("/abc", $path);
Copy link
Contributor Author

@weakish weakish Dec 11, 2020

Choose a reason for hiding this comment

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

正常情况在这里，上面两个分支是为了兼容国际版很老的应用以及比较新的使用饕餮之桶的应用。

@weakish weakish requested a review from juvenn December 11, 2020 12:53
juvenn
juvenn approved these changes Dec 14, 2020
View reviewed changes
Copy link
Member

@juvenn juvenn left a comment

Choose a reason for hiding this comment

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

👍

.github/workflows/php.yml
LEANCLOUD_API_SERVER: https://us.avoscloud.com
LEANCLOUD_APP_ID: wnDg0lPt0wcYGJSiHRwHBhD4
LEANCLOUD_APP_KEY: u9ekx9HFSFFBErWwyWHFmPDy
LEANCLOUD_APP_MASTER_KEY: ${{ secrets.MASTER_KEY }}
Copy link
Member

@juvenn juvenn Dec 14, 2020

Choose a reason for hiding this comment

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

是的，之前是在 ci 上面设置的环境变量。

@weakish weakish merged commit 2abbfd3 into master Dec 14, 2020
@weakish weakish deleted the deprecate-file-api branch December 14, 2020 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juvenn juvenn juvenn approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@weakish @juvenn