Skip to content

Commit

Permalink
code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@rdebleu
rdebleu committed Oct 13, 2023
1 parent f7af2f4 commit c14061f
Show file tree
Hide file tree
Showing 12 changed files with 105 additions and 95 deletions.
24 changes: 12 additions & 12 deletions .github/workflows/cron.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,52 +52,52 @@ jobs:
MOODLE_BRANCH: ${{ matrix.moodle-branch }}

- name: phplint
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi phplint

- name: phpcpd
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi phpcpd

- name: phpmd
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi phpmd

- name: phpdoc
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi phpdoc

- name: codechecker
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi codechecker

- name: validate
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi validate

- name: savepoints
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi savepoints

- name: grunt
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi grunt

- name: mustache
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi mustache

- name: phpunit
continue-on-error: true
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi phpunit --coverage-text --coverage-clover

- name: behat
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi behat --profile=chrome

- name: coveralls
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci coveralls-upload
env:
COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
36 changes: 18 additions & 18 deletions .github/workflows/main.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
strategy:
fail-fast: false
matrix:
moodle-branch: ['MOODLE_401_STABLE', 'MOODLE_402_STABLE', 'master']
moodle-branch: ['MOODLE_401_STABLE', 'MOODLE_402_STABLE', 'MOODLE_403_STABLE', 'master']
php: ['8.1']
database: ['mariadb', 'pgsql']
include:
Expand Down Expand Up @@ -60,7 +60,7 @@ jobs:
with:
php-version: ${{ matrix.php }}
ini-values: max_input_vars=5000
coverage: xdebug
coverage: pcov

- name: composer
run: |
Expand All @@ -80,68 +80,68 @@ jobs:
working-directory: moodle/local/aws/sdk/GuzzleHttp

- name: phplint
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi phplint

- name: phpcpd
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi phpcpd

- name: phpmd
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi phpmd

- name: phpdoc
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi phpdoc

- name: codechecker
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi codechecker

- name: validate
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi validate

- name: savepoints
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi savepoints

- name: grunt
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi grunt

- name: mustache
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi mustache

- name: phpunit
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi phpunit --coverage-text --coverage-clover

- name: privacy
run: vendor/bin/phpunit --colors --no-coverage --testsuite tool_dataprivacy_testsuite,tool_policy_testsuite,core_privacy_testsuite
if: ${{ always() }}
if: ${{ !cancelled() }}
working-directory: moodle

- name: firefox
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi behat

- name: chrome
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi behat --profile chrome

- name: firefox classic
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi behat --suite classic

- name: chrome classic
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci --ansi behat --suite classic --profile=chrome

- name: coveralls
if: ${{ always() }}
if: ${{ !cancelled() }}
run: moodle-plugin-ci coveralls-upload
env:
COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
8 changes: 4 additions & 4 deletions db/access.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@

$capabilities = [
'repository/s3bucket:view' => ['captype' => 'read', 'contextlevel' => CONTEXT_MODULE,
'archetypes' => ['editingteacher' => CAP_ALLOW, 'manager' => CAP_ALLOW]],
'archetypes' => ['editingteacher' => CAP_ALLOW, 'manager' => CAP_ALLOW], ],
'repository/s3bucket:addinstance' => ['captype' => 'write', 'contextlevel' => CONTEXT_SYSTEM,
'archetypes' => ['manager' => CAP_ALLOW]],
'archetypes' => ['manager' => CAP_ALLOW], ],
'repository/s3bucket:addinstance' => ['captype' => 'write', 'contextlevel' => CONTEXT_USER,
'archetypes' => ['editingteacher' => CAP_ALLOW, 'manager' => CAP_ALLOW]],
'archetypes' => ['editingteacher' => CAP_ALLOW, 'manager' => CAP_ALLOW], ],
'repository/s3bucket:addinstance' => ['captype' => 'write', 'contextlevel' => CONTEXT_COURSE,
'archetypes' => ['editingteacher' => CAP_ALLOW, 'manager' => CAP_ALLOW]]];
'archetypes' => ['editingteacher' => CAP_ALLOW, 'manager' => CAP_ALLOW], ], ];
22 changes: 11 additions & 11 deletions lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function get_listing($path = '.', $page = 1) {
'Prefix' => $path,
'MaxKeys' => 1000,
'EncodingType' => 'url',
'Delimiter' => '/'];
'Delimiter' => '/', ];
$results = $files = [];
$s3 = $this->create_s3();
try {
Expand All @@ -75,7 +75,7 @@ public function get_listing($path = '.', $page = 1) {
'thumbnail' => $diricon,
'thumbnail_height' => 64,
'thumbnail_width' => 64,
'path' => $item['Prefix']];
'path' => $item['Prefix'], ];
}

$filesearch = 'Contents[?StorageClass != \'DEEP_ARCHIVE\'';
Expand All @@ -93,7 +93,7 @@ public function get_listing($path = '.', $page = 1) {
'thumbnail_height' => 64,
'thumbnail_width' => 64,
'source' => $item['Key'],
'thumbnail' => $OUTPUT->image_url(file_extension_icon($pathinfo['basename'], 64))->out(false)];
'thumbnail' => $OUTPUT->image_url(file_extension_icon($pathinfo['basename'], 64))->out(false), ];
}
}
return [
Expand All @@ -102,7 +102,7 @@ public function get_listing($path = '.', $page = 1) {
'manage' => false,
'dynload' => true,
'nologin' => true,
'nosearch' => false];
'nosearch' => false, ];
}

/**
Expand All @@ -121,7 +121,7 @@ public function search($q, $page = 1) {
'FetchOwner' => false,
'MaxKeys' => 1000,
'EncodingType' => 'url',
'Delimiter' => '/'];
'Delimiter' => '/', ];
$results = $files = [];
$s3 = $this->create_s3();
try {
Expand All @@ -138,7 +138,7 @@ public function search($q, $page = 1) {
'thumbnail' => $diricon,
'thumbnail_height' => 64,
'thumbnail_width' => 64,
'path' => $item['Prefix']];
'path' => $item['Prefix'], ];
}

$filesearch = 'Contents[?StorageClass != \'DEEP_ARCHIVE\'';
Expand All @@ -155,7 +155,7 @@ public function search($q, $page = 1) {
'thumbnail_height' => 64,
'thumbnail_width' => 64,
'source' => $item['Key'],
'thumbnail' => $OUTPUT->image_url(file_extension_icon($pathinfo['basename'], 64))->out(false)];
'thumbnail' => $OUTPUT->image_url(file_extension_icon($pathinfo['basename'], 64))->out(false), ];
}
return ['list' => $files, 'dynload' => true, 'pages' => 0, 'page' => $page];
}
Expand Down Expand Up @@ -186,7 +186,7 @@ public function send_otherfile($reference, $lifetime) {
$options = [
'Bucket' => $this->get_option('bucket_name'),
'Key' => $reference,
'ResponseContentDisposition' => 'attachment'];
'ResponseContentDisposition' => 'attachment', ];
try {
$result = $s3->getCommand('GetObject', $options);
$req = $s3->createPresignedRequest($result, $lifetime);
Expand Down Expand Up @@ -251,7 +251,7 @@ public function get_file($filepath, $file = '') {
$options = [
'Bucket' => $this->get_option('bucket_name'),
'Key' => $filepath,
'SaveAs' => $path];
'SaveAs' => $path, ];
try {
$s3->getObject($options);
} catch (\Exception $e) {
Expand Down Expand Up @@ -391,7 +391,7 @@ private function create_s3() {
$arr = self::addproxy([
'credentials' => ['key' => $accesskey, 'secret' => $this->get_option('secret_key')],
'use_path_style_endpoint' => true,
'region' => $this->get_option('endpoint')]);
'region' => $this->get_option('endpoint'), ]);
$this->_s3client = \Aws\S3\S3Client::factory($arr);
}
return $this->_s3client;
Expand Down Expand Up @@ -419,7 +419,7 @@ private static function addproxy($settings) {
$day = new DateTime();
$result = new \Aws\Result([
'CommonPrefixes' => [['Prefix' => '2020_dir']],
'Contents' => [['Key' => '2020_f.jpg', 'Size' => 15, 'StorageClass' => 'STANDARD', 'LastModified' => $day]]]);
'Contents' => [['Key' => '2020_f.jpg', 'Size' => 15, 'StorageClass' => 'STANDARD', 'LastModified' => $day]], ]);
$mock->append($result, $result);
$settings['handler'] = $mock;
}
Expand Down
41 changes: 0 additions & 41 deletions phpunit.xml
View file

This file was deleted.

2 changes: 1 addition & 1 deletion tests/delete_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function test_deletebucket() {
$this->assertEquals($cnt + 1, $DB->count_records('repository_instances'));
$fs = get_file_storage();
$filerecord = ['component' => 'user', 'filearea' => 'draft', 'contextid' => \context_user::instance($USER->id)->id,
'itemid' => file_get_unused_draft_itemid(), 'filename' => $reference, 'filepath' => '/'];
'itemid' => file_get_unused_draft_itemid(), 'filename' => $reference, 'filepath' => '/', ];
$this->assertEquals(0, $DB->count_records('files_reference'));
$fs->create_file_from_reference($filerecord, $repoid, $reference);
$this->assertEquals(1, $DB->count_records('files_reference'));
Expand Down
4 changes: 2 additions & 2 deletions tests/form_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function setUp(): void {
'endpoint' => 'eu-west-2',
'secret_key' => 'secret',
'bucket_name' => 'test',
'access_key' => 'abc'];
'access_key' => 'abc', ];
$this->SetAdminUser();
}

Expand All @@ -80,7 +80,7 @@ public function test_type_config_form() {
'plugin' => 's3bucket',
'action' => 'show',
'pluginname' => 'repository_s3bucket',
'contextid' => $context->id];
'contextid' => $context->id, ];
$mform = new \repository_type_form('', $para);
$this->assertEquals([], \repository_s3bucket::type_form_validation($mform, null, []));
ob_start();
Expand Down
2 changes: 1 addition & 1 deletion tests/generator/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ protected function prepare_record(array $record) {
'access_key' => 'access',
'secret_key' => 'secret',
'endpoint' => 'us-east-1',
'bucket_name' => 'testrepo'];
'bucket_name' => 'testrepo', ];
return array_merge($arr, $record);
}
}
2 changes: 1 addition & 1 deletion tests/mock_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function test_listobjects() {
'Prefix' => '.',
'MaxKeys' => 1000,
'EncodingType' => 'url',
'Delimiter' => '/'];
'Delimiter' => '/', ];
$client = $this->getMockBuilder('Aws\S3\S3Client')
->disableOriginalConstructor()
->setMethods(['listObjectsV2', 'getPaginator', 'getObject'])
Expand Down
6 changes: 3 additions & 3 deletions tests/other_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function setUp(): void {
'endpoint' => 'eu-north-1',
'secret_key' => 'secret',
'bucket_name' => 'test',
'access_key' => 'abc'];
'access_key' => 'abc', ];
$this->SetAdminUser();
}

Expand All @@ -82,7 +82,7 @@ public function test_sendfiles3() {
$repo = new \repository_s3bucket($this->repo);
$fs = get_file_storage();
$filerecord = ['component' => 'user', 'filearea' => 'draft', 'contextid' => \context_user::instance($USER->id)->id,
'itemid' => file_get_unused_draft_itemid(), 'filename' => 'filename.jpg', 'filepath' => '/'];
'itemid' => file_get_unused_draft_itemid(), 'filename' => 'filename.jpg', 'filepath' => '/', ];
$file = $fs->create_file_from_string($filerecord, 'test content');
$this->expectException('repository_exception');
$repo->send_file($file);
Expand Down Expand Up @@ -184,7 +184,7 @@ public function test_getfile() {
$repo->set_option($this->data);
$draft = file_get_unused_draft_itemid();
$filerecord = ['component' => 'user', 'filearea' => 'draft', 'contextid' => $context->id,
'itemid' => $draft, 'filename' => 'filename.txt', 'filepath' => '/'];
'itemid' => $draft, 'filename' => 'filename.txt', 'filepath' => '/', ];
get_file_storage()->create_file_from_string($filerecord, 'test content');
$result = $repo->get_file('/filename.txt');
$this->assertEquals('/filename.txt', $result['url']);
Expand Down
Loading

0 comments on commit c14061f

Please sign in to comment.