Media firewall 2: Differentiate between mediaRoot/mediaDir

[lukasbestle]

Description

Summary of changes

• New custom $filename param for $file->mediaRoot() and $file->mediaUrl()
• New $file->mediaDir() method (= directory where media of that file model is stored)
• Rename mediaRoot() to mediaDir() for other model classes

Reasoning

• With the new media path structure introduced by Media firewall 3: Introduce media subhashes #7195, methods throughout the core need access to both the "topmost" media root and the individual file paths.
• Being able to pass a custom filename to mediaRoot() and mediaUrl() allows to clean up thumb-related code and is also needed for the media firewall PR 3 Media firewall 3: Introduce media subhashes #7195.

Changelog

Enhancements

• New $file->mediaDir() method that returns the directories where media of that file model is stored. Matching method aliases for the Page, Site and User classes are provided.
• The $file->mediaRoot() and $file->mediaUrl() methods now accept a new optional $filename argument so they can be used to construct roots and URLs to thumbs.

Breaking changes

None

Docs

None

Ready?

• In-code documentation (wherever needed)
• Unit tests for fixed bug/feature
• Tests and CI checks all pass

For review team

• Add changes & docs to release notes draft in Notion

[distantnative]

To me the breaking change is worth it as the naming scheme would make a lot of sense this way.

Would be good if @bastianallgeier also has a look at this.

New solution discussed on Discord

[bastianallgeier]

Changes

• I've created a new ::mediaDir method for all models.
• ::mediaRoot is now an alias for ::mediaDir in the Page, Site and User classes
• ::mediaRoot has a new optional $filename param in the File class
• ::mediaPath has been removed again in favor of the existing ::mediaRoot
• All unit tests have been adapted.

@lukasbestle please check if you are happy with those changes.

[lukasbestle]

Thank you for adapting it, looks good to me. Unfortunately I cannot approve my own PR :)

One thing I thought about is whether we want to deprecate $page/site/user->mediaRoot() with a deprecation warning. Just to clean it all up and avoid accumulating aliases that aren't really needed. Especially with our plan to introduce media subclasses (which mean we need to deprecate all those media methods in the model classes anyway).

[lukasbestle]

Another thought on the deprecation: We could actually introduce the media subclasses in v5.x and deprecate all old methods in one go then. This avoids having to migrate calls to the new methods twice.

Which means we can merge this as is. If we want, we could already add @deprecated tags to the old methods without deprecation warnings, but it’s not required IMO.

[bastianallgeier]

@lukasbestle sounds good to me!