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

Folders structure for media RFC #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

precious-void
Copy link

@precious-void precious-void commented Mar 30, 2021

See rfc here
RFC is referred to strapi/strapi#8612
Waiting for you comments on this topic!

@strapi-cla
Copy link

strapi-cla commented Mar 30, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Artem Shteltser seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@precious-void precious-void changed the title Folders structure for media Folders structure for media RFC Mar 30, 2021
@alexandrebodin
Copy link
Member

Hi, thank you for this RFC.

At first glance I am wondering why the folder model would be necessary ?

I think have a simple path key in the File model that would represent a structure path like '/dir1/subDir' would be a simple solution to implement virutal folders in the Admin

It would be farily trivial to list the folders / move files around/ deleting everything in a folder etc...

We could event use this to serve as a prefix on hosted service & in the static server that is serving the files.

@derrickmehaffy
Copy link
Member

Hi, thank you for this RFC.

At first glance I am wondering why the folder model would be necessary ?

I think have a simple path key in the File model that would represent a structure path like '/dir1/subDir' would be a simple solution to implement virutal folders in the Admin

It would be farily trivial to list the folders / move files around/ deleting everything in a folder etc...

We could event use this to serve as a prefix on hosted service & in the static server that is serving the files.

I was wondering the same but it has been requested with specific providers (like AWS) and we already support it on at least one provider.

I think if it doesn't raise the difficulty too much, why not but would probably need to be explored.

@precious-void
Copy link
Author

precious-void commented Mar 30, 2021

@derrickmehaffy @alexandrebodin to be honest I feel a bit confused about the way of listing files using path key. I mean, to list all folders you will need to iterate over all of the files to get all the folders we have, with subfolders much more unnecessary executions. Also the creation of the folders is a question. I mean, folders can be empty and according to what you suggest, it cannot be. Am I right?

@alexandrebodin
Copy link
Member

alexandrebodin commented Mar 31, 2021

Hi, Yes I don't see the need for empty folders if we say they are "virtual". If we allow creating folder only when you do a move action then it is unnecessary.

For the operations it is actually way simpler than iterating over all the files. We can do very simple DB operations to retrieve the list of folders that should be displayed at anytime. with simple indexing this would be very efficient. On the contrary using a relation for folder is less efficient as it means finding files by a relation which requires more processing

Here are some dumb queries that would do the job.

-- select files in a folder is super easy
SELECT * FROM test where path LIKE '/sub2%';


-- listing all folder and counts or other aggegations
SELECT path, count(*) FROM test GROUP BY path;


-- super easy to create the file structure with this in the ui
SELECT path, count(*) 
FROM test 
GROUP BY path
ORDER BY path, LENGTH(path);

-- find folder of x depth
SELECT path, count(*), LENGTH(path) - LENGTH(replace(path, '/', '')) - 1 as depth
FROM test 
GROUP BY path
HAVING depth = 0
ORDER BY path, LENGTH(path);

Depending on what our product managers are planning for the folder features it will drive how we implement it anyway :) I'll wait their input first :)

@precious-void
Copy link
Author

Hi, thank you for this RFC.

At first glance I am wondering why the folder model would be necessary ?

I think have a simple path key in the File model that would represent a structure path like '/dir1/subDir' would be a simple solution to implement virutal folders in the Admin

It would be farily trivial to list the folders / move files around/ deleting everything in a folder etc...

We could event use this to serve as a prefix on hosted service & in the static server that is serving the files.

Hah, yeah really dumb queries :D
In general, it looks fine. I'm not sure it's as good as my solution in terms of execution time and efficiency, especially in mongo. Correct me if I'm wrong.

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.

4 participants