In multisite, the "storage space used" is incorrectly calculated for the main blog/site.

[rebekahickey]

Recently identified an issue in a few multisite installs where the "space used" that displays on the dashboard of the main site is calculating everything in /wp-content/uploads/, including the subsite uploads in /wp-content/uploads/sites/. For instance, the main site has only 4MB in its Media Library, but the dashboard shows that it's exceeded the storage limit of 150MB and prevents users from uploading files. The additional storage used corresponds with the files uploaded to the network's subsites in /wp-content/uploads/sites/.

Notes:

• This is not an issue on other multisite instances that are using the /blogs.dir/ path for uploads.
• Organize uploads by year/month is disabled on all instances.

[earnjam]

In 30202.diff​

• Added optional $exclude parameter to get_dirsize() to allow the exclusion of a directory from the total
• Added optional $exclude parameter to recurse_dirsize() to allow the exclusion of a directory from the total
• In get_space_used() added check for is_main_site(), and if true, pass the uploads/sites sub-directory as the exclude value to get_dirsize() so that we are only counting the files for the main site from the main site dashboard.

I also cleaned up the individual edited lines to match the coding standards.

[jeremyfelt]

Thanks for the ticket @rebekahickey!

@earnjam is working on some tests for the above patch as well.

[jeremyfelt]

We should get some unit tests on this before it goes in. Overall, it's a relatively low impact issue (main site only), though very annoying for anyone experiencing it. Let's go for 4.3.

[earnjam]

Just thought about a scenario here...since we cache the return from get_dirsize() as a transient, it's possible that if it was called directly outside of core with the new exclude parameter passed to it, it would cache that value instead of the expected core usage.

Not sure the best way to prevent that.

Perhaps don't cache the value if the exclude parameter is passed to it unless it is for the main site uploads directory?

[earnjam]

30202.tests.diff​ is some tests for get_space_used()

30202.1.diff​ adds in some additional checks to prevent caching or serving a cached value when the exclude parameter is passed unless it is on the main site. This way we don't end up with incorrect values.

[earnjam]

Ah, the diff on 320202.1.diff is kinda messy to read. I probably shouldn't have made the whitespace changes inside the get_dirsize() and recurse_dirsize() functions. I'll add another patch with just the necessary functional changes if needed.

[wonderboymusic]

30202.2.diff​ is a refresh but 4 unit tests fail. This sucks because it is hard to toggle the value for blog_upload_space and then reset it to the proper value between tests while taking into account attachments from src install.

[obenland]

Can we get this finished before beta2 or should we punt?

[jeremyfelt]

I'm going to take one more close look at this in the next day or so. That testing is tough is a worry, but we'll figure out something. :)

[jeremyfelt]

Replying to earnjam:

Just thought about a scenario here...since we cache the return from get_dirsize() as a transient, it's possible that if it was called directly outside of core with the new exclude parameter passed to it, it would cache that value instead of the expected core usage.

Not sure the best way to prevent that.

Perhaps don't cache the value if the exclude parameter is passed to it unless it is for the main site uploads directory?

I went with this as well in the latest patch, but I'm thinking we may just want to leave it alone. get_dirsize() has always returned the cached value when requested, so it's a very specific function and use for anything else would lead to strange results in general. If we want to make this more accessible to developers, there's probably a better way to do it.

Replying to wonderboymusic:

30202.2.diff​ is a refresh but 4 unit tests fail. This sucks because it is hard to toggle the value for blog_upload_space and then reset it to the proper value between tests while taking into account attachments from src install.

I've gotten around this in 30202.3.diff.

• Moved the tests to their own file to match the pattern we've been using elsewhere recently.
• New sites are created until it is detected that we have an empty uploads directory for that site.
• We then know it's safe to remove the folder and its contents entirely when the test is complete.
• The main site can be handled directly in the individual test by checking space used before any new site is created.

I'm going to think through this a bit more and make some of the transient changes. Should be close.

[jeremyfelt]

Replying to jeremyfelt:

get_dirsize() ..... If we want to make this more accessible to developers, there's probably a better way to do it.

I think the right answer now is to not add $exclude to get_dirsize() and allow it to function as normal. We can then detect is_main_site() there and pass exclude to recurse_dirsize() if needed. If a developer wanted to retrieve the size of a directory while excluding another, recurse_dirsize() would be available, though without caching.

If we add $exclude to get_dirsize(), it should go hand in hand with #19879.

[jeremyfelt]

In 33184:

Exclude individual site directories when calculating space used for the main site.

• Add an $exclude parameter to recurse_dirsize().
• Use this parameter in get_dirsize() to exclude /sites when on the main site.
• Add tests for main site and switched site.

Props @earnjam, @jeremyfelt.
Fixes #30202.

[jeremyfelt]

In 33185:

Add explicit testing for pre_get_space_used() filter.

See #30202.