does it work? - Githubissues

durzo commented 4 years ago

I just installed this plugin to find it doesn't properly list objects in the bucket.

The code in lib.php get_listing() looks like it would never work with what the S3 ListObjects API returns.

The result is I get an empty filepicker window.

If this plugin is unmaintained, I would be keen to get it working again.

ewallah commented 4 years ago

Hello Durzo,

This plugin works fine in Moodle 3.6 to Moodle3.8. Please review your error logs and configuration if your filepicker remains empty.

R

durzo commented 4 years ago

Plugin is installed and configured fine nothing in error log Narrowing the problem down to it not showing directories, only files.

my bucket listing:

$ aws s3 ls s3://mybucket/
                           PRE folder-test/
2020-05-10 21:22:08          6 readme.txt

Filepicker shows the file 'readme.txt' but not the directory 'folder-test'.

The code I was referencing in lib.php:

        foreach ($results as $result) {
            foreach ($result['Contents'] as $object) {
                $pathinfo = pathinfo($object['Key']);
                if ($object['Size'] == 0) {
                    if ($pathinfo['dirname'] == $path) {
                        $files[] = ['title' => $pathinfo['basename'], 'children' => [], 'thumbnail' => $diricon,
                                    'thumbnail_height' => 24, 'thumbnail_width' => 24, 'path' => $object['Key']];
                    }
                } else {
                    if ($pathinfo['dirname'] == $path or $pathinfo['dirname'] . '//' == $path) {
                        $files[] = ['title' => $pathinfo['basename'], 'size' => $object['Size'], 'path' => $object['Key'],
                                    'datemodified' => date_timestamp_get($object['LastModified']), 'thumbnail_height' => 24,
                                    'thumbnail_width' => 24, 'source' => $object['Key'], 'thumbnail' => $fileicon];
                    }
                }
            }
        }

Specifically this line:

                if ($object['Size'] == 0) {

the ListObjects API doesnt return a Size of 0 for directories. so this will never work

ewallah commented 4 years ago

Perhaps a picture can say a 1000 words:

capture_1

durzo commented 4 years ago

$ aws --profile personal s3 ls s3://m3bucket/ --recursive
2020-03-06 15:15:12         21 00/00/0000000000000000000000000000000000000011_devtest
2020-03-06 15:16:35         21 00/00/0000000000000000000000000000000000000012_devtest
2020-03-06 15:21:33   70463248 00/00/0000000000000000000000000000000000000013_devtest
2020-03-06 15:56:59         24 00/00/0000000000000000000000000000000000123456_devtest
2020-03-04 14:21:14       8202 10/55/10554f0a5f011d1f5ab2c22fe900f8257eb16f4b_devtest
2020-03-04 14:21:14      74541 25/c6/25c6b93bb104f67ffbfce121de09e3a7e629f9c6_devtest
2020-03-08 12:44:23   17297227 35/28/3528469f5c79a1da0e0b647506427795cd40d227_devtest
2020-03-04 14:21:15     211437 4a/45/4a45d10ca74fbc472bfe7450e13a875a77d6ad49_devtest
2020-05-09 18:51:40     121647 52/a7/52a746466a65e2dcdcede4f91bbf220cd971d5c5_devcopy
2020-03-07 16:07:47     121647 52/a7/52a746466a65e2dcdcede4f91bbf220cd971d5c5_devtest
2020-03-08 12:43:07   17297224 55/59/55591306f4d618751874437bad9b949551a7f8a6_devtest
2020-03-04 14:21:14      16121 5e/f2/5ef234e1bf5ec3a924c246e82a98021c77b96864_devtest
2020-03-04 14:21:14      12335 5f/34/5f3410bbaf46604561f759a2a9b2ee90dc1a7b75_devtest_devtest
2020-04-09 19:43:32       9656 85/66/856654b7a2e9cedb26c0d8bacc8fd7235d7fdd16_devtest
2020-04-06 15:28:05      23162 86/37/86379ab8e75d35759b2e3e1a8cd1e752d13abd28_devtest
2020-03-08 12:29:28   17297280 88/b9/88b901bad0a67bffcfcefb805d4673a25ed8b84f_devtest
2020-05-09 22:54:26       2332 autobackups/backup-moodle2-course-1-20200509-2254.mbz
2020-05-09 22:54:21   17297196 autobackups/backup-moodle2-course-2-20200509-2254.mbz
2020-05-11 11:56:20         12 autobackups/file1.txt
2020-05-10 22:50:51          6 autobackups/findme/hello.txt
2020-05-11 11:56:20         12 autobackups/subdir1/subdir1_2/hellothere.txt
2020-05-11 11:56:20         12 autobackups/subdir1/subdir1_2/subdir_anotherone/hellothere_again.txt
2020-05-11 11:56:20         12 autobackups/subdir1/subdir1_file1.txt
2020-05-11 11:56:20         12 autobackups/subdir1/subdir1_file2.txt
2020-05-11 11:56:20         12 autobackups/subdir2/subdir1_file1.txt
2020-05-09 18:51:28         63 autobackups/warning.txt
2020-03-06 18:54:07         24 c5/19/c5193953d3ff2ba3c6400bffd50f294033ffdb83_devtest
2020-05-09 18:51:41   17168343 c5/3b/c53b42247d39d7b89b6e2891be9459dc6285db90_devcopy
2020-03-08 12:26:33   17168343 c5/3b/c53b42247d39d7b89b6e2891be9459dc6285db90_devtest
2020-03-04 14:21:15     148602 c6/75/c6750f347464948ac83834fa843d0cab60aa1124_devtest_devtest
2020-05-09 18:51:40      35236 c6/89/c68994bc9b714474dd795d92d2cab34fc16aef7b_devcopy
2020-03-04 14:21:14      35236 c6/89/c68994bc9b714474dd795d92d2cab34fc16aef7b_devtest
2020-03-04 14:21:14      35236 c6/89/c68994bc9b714474dd795d92d2cab34fc16aef7b_devtest_devtest
2020-03-06 19:17:23         28 ce/eb/ceebcced779b65e2b561600056c82224f1d7220c_devtest
2020-03-08 12:34:21   17297221 d2/50/d250e304aac5406398775c08cdb14ac6c2be406f_devtest
2020-03-08 12:37:38   17297222 d6/fe/d6fe65056faaea4da1b44f89b1f428c13af6a666_devtest
2020-05-11 12:54:55      12252 da/89/da89b3abd63517f5c31b0127e20be1d2b293148f_devcopy
2020-05-11 12:54:53     102866 de/ae/deaeb285425e39724e44723c9b252e2fa027af2a_devcopy
2020-05-11 12:54:55      11221 df/b4/dfb42bc28020e23683e26af6f3bb257095337afa_devcopy
2020-03-07 16:07:49      11221 df/b4/dfb42bc28020e23683e26af6f3bb257095337afa_devtest
2020-03-04 14:21:14      14043 f3/51/f351c6c712c91198e70d449c6e505f06d69818aa_devtest_devtest
2020-04-09 19:43:30      66509 fc/d3/fcd3e5508b8b87628f2e43becf0f5f9b55f8227b_devtest
2020-05-10 21:22:08          6 readme.txt

s3bucket

If I throw in an error_log to capture the object sizes, none of them ever match 0 bytes. but this is returned by the S3 API on ListObjects.. I am using the local_aws plugin with the most up to date SDK version - 3.137.6 - 2020-05-08

Can I ask what SDK version you are using?

ewallah commented 4 years ago

My output from the AWS Command Line Interface (cli) is showing folders with size 0:

#aws cli s3 ls s3://xxx --recursive
2020-05-11 10:48:27      40960 Request for extension due to Covid-19.doc
2020-05-11 11:02:51          0 folder/
2020-05-11 11:03:37      93979 folder/Request for extension due to Covid-19.pdf

The same when I use the S3 management console, S3 Browser or WinSCP: I see files/folders/subfolders.

If your AWS cli does not see folders, then my plugin using AWS SDK 3.134.6 - 2020-04-08 will behave similar. If there is no folder and subfolder info available in the bucket, then the plugin does not show folders and subfolders.

How did you added the non root files to your bucket?

durzo commented 4 years ago

I added files via the s3 command. heres what I did:

$ mkdir s3test
$ mkdir -p s3test/{1,2,3}
$ echo test > s3test/1/test.txt
$ echo test > s3test/2/test.txt
$ echo test > s3test/3/test.txt
$ aws s3 cp s3test/ s3://mybucket/s3test/ --recursive
upload: s3test/1/test.txt to s3://mybucket/s3test/1/test.txt
upload: s3test/2/test.txt to s3://mybucket/s3test/2/test.txt
upload: s3test/3/test.txt to s3://mybucket/s3test/3/test.txt
$ aws s3 ls s3://mybucket/s3test/
                           PRE 1/
                           PRE 2/
                           PRE 3/
$ aws s3 ls s3://mybucket/s3test/ --recursive
2020-05-11 18:56:08          5 s3test/1/test.txt
2020-05-11 18:56:08          5 s3test/2/test.txt
2020-05-11 18:56:08          5 s3test/3/test.txt

Also using the s3api to directly execute a list-objects doesnt even show the directories, only the files inside them - this is what im seeing when i error_log() the list-objects command from the plugin.

$ aws s3api list-objects --bucket mybucket --prefix s3test --output json
{
    "Contents": [
        {
            "LastModified": "2020-05-11T16:56:08.000Z", 
            "ETag": "\"d8e8fca2dc0f896fd7cb4cb0031ba249\"", 
            "StorageClass": "STANDARD", 
            "Key": "s3test/1/test.txt", 
            "Owner": {
                "DisplayName": "durzo", 
                "ID": "hidden"
            }, 
            "Size": 5
        }, 
        {
            "LastModified": "2020-05-11T16:56:08.000Z", 
            "ETag": "\"d8e8fca2dc0f896fd7cb4cb0031ba249\"", 
            "StorageClass": "STANDARD", 
            "Key": "s3test/2/test.txt", 
            "Owner": {
                "DisplayName": "durzo", 
                "ID": "hidden"
            }, 
            "Size": 5
        }, 
        {
            "LastModified": "2020-05-11T16:56:08.000Z", 
            "ETag": "\"d8e8fca2dc0f896fd7cb4cb0031ba249\"", 
            "StorageClass": "STANDARD", 
            "Key": "s3test/3/test.txt", 
            "Owner": {
                "DisplayName": "durzo", 
                "ID": "hidden"
            }, 
            "Size": 5
        }
    ]
}

ewallah commented 4 years ago

AWS sync, cp and mv indeed do NOT create folder objects.

Folders that were created by S3 management console, S3 Browser, WinSCP or manualy using: aws s3api put-object --bucket bucketname --key bucketfolder/ have size 0 and are listed.

As far as I know, this is related to the impossibiliy to create subdirectories in a bucket. There is even a manual available when someone is advised to create a dummy file in a bucket - rename the file - ...

So the problem can be reduced to:

should the plugin create the missing folders so also other tools see these folders?
or should the plugin be compatible with not properly created folders?
or should the end user create the missing folders?

The first option is not available as this plugin does not knows the correct encryption setting to create these subfolders.

R

durzo commented 4 years ago

By using Delimiter in the list-objects call I have managed to make it work with both types of directory creation structure.

While I was at it I also added a few other things we needed:

Allow use of IAM Roles by not specifying access/secret keys.
Rewrite most of get_listing() to better identify files & (pseudo) directories
Add chroot base path option to restrict to certain directories within the bucket.
Force files to be served as download attachments via the S3 Signed URL
Fixed supported return types - ability to make copy or alias
Changed repo icon to the official amazon S3 icon.
Added subdir breadcrumbs to the top of the picker.

The changes can be found in my repo here: https://github.com/durzo/moodle-repository_s3bucket/commit/db8514adf03f6dfecfd625baedaea69ba2f2f0b4

I understand you may consider some of these things out of scope or unwanted in your plugin, which is why I forked it. I am happy to split the features into separate, smaller commits if you would like PR's for them.

Screenshot from 2020-05-11 22-14-27

Screenshot from 2020-05-11 22-12-08

Screenshot from 2020-05-11 22-12-18

ewallah commented 4 years ago

Your commits are making this plugin only more complex without providing good documentation.

durzo commented 4 years ago

sorry fixing your bugs and adding functionality makes it more complex. happy to fork this and maintain my own version. peace :)

Hookz commented 4 years ago

sorry fixing your bugs and adding functionality makes it more complex. happy to fork this and maintain my own version. peace :)

Hey @durzo ! Are you going to keep updating the plugin or make a separate plugin for Moodle fixing the issue? I ran into it a couple of weeks ago and couldn't fix it until I found this thread. I cloned your version of the repo and it works perfectly! but Moodle started shouting at me that I'm using a version below the developer's one and changing the code to fake the version every time is painful… And as we see the main dev refuses to fix it…

durzo commented 4 years ago

Hi @Hookz,

I do plan on maintaining my fork and renaming it has been on my todo list.

Tomorrow I will change the plugin name - I'm not aware of anything else that needs fixing, but if you come across something please open an issue on my repo so we can leave ewallah alone :)

ewallah / moodle-repository_s3bucket

does it work? #3