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

[5.2] Rebuild the image filter to a registry allowing custom filters outside the main namespace #31818

Open
wants to merge 8 commits into
base: 5.2-dev
Choose a base branch
from

Conversation

wilsonge
Copy link
Contributor

Pull Request for Issue #31804 .

Summary of Changes

Refactor the image filter class to have custom namespaces by using a registry for loading classes.

Why is this b/c changing at this stage?

Because there is a poor user experience for third party extensions who'd have to custom autoload code to add custom filters (as evidenced by the fact we were doing this in our own tests until yesterday)

Testing Instructions

  • Check loading a custom image class with a custom namespace
  • Check drone tests continue pass when loading a custom registry

Documentation Changes Required

Yup - refactor the namespaced stuff.

@wilsonge wilsonge changed the title Rebuild the image filter to a registry allowing custom filters outside the main namespace [4.0] Rebuild the image filter to a registry allowing custom filters outside the main namespace Dec 31, 2020
libraries/src/Image/Image.php Outdated Show resolved Hide resolved
Co-authored-by: Quy <quy@fluxbb.org>
@soockee
Copy link

soockee commented Aug 17, 2021

Tried to create a custom image and add a custom filter to the Registry of the CustomImage class, which is in another namespace, e.g. Joomla\CMS\Customimage
This seems to work, but is it intended behavior that the Image class and the CustomImage class share the registry and all their newly registered filters?

Inside a Plugin in /plugins/system/

                $image = new CustomImage(imagecreatetruecolor(1, 1));
		// Verify that the filter type exists.
		$serviceRegistry =  CustomImage::getServiceRegistry();
		if(!$serviceRegistry->hasService($type)){
			$serviceRegistry->register($type, SuperBrightness::class);
		}
		$className = $this->getClassName($type, $serviceRegistry, CustomImage::class);

		// Instantiate the filter object.
		$instance = new $className($image->getHandle());

		if(!$this->isValid($instance, SuperBrightness::class)){
			throw new \RuntimeException('The ' . ucfirst($type) . ' image filter is not valid.');
		}

I also tried to replace the registry of the Image class with a custom Registry in another namespace, The Image class expects the registry to be of type \Joomla\CMS\Image\ImageFilterRegistry and therefore it seems no custom registry ( that is in another namespace ) can be provided.
Is it intended to not be able to overwrite the Registry of the Image class?
I imaging that my custom registry provides different( e.g. custom filters ) for already existing filter keys like

'brightness' => Filter\CustomBrightness::class,

registry replacement snippet, in which the custom registry provides a custom brightness filter, but fails

                Factory::getContainer()->set(\Joomla\CMS\Image\ImageFilterRegistry::class,new \Joomla\CMS\Customimage\ImageFilterRegistry); 
		$options[IMG_FILTER_BRIGHTNESS] = 50;
		$image = new Image(imagecreatetruecolor(1, 1));

                // This does not work, because the Image class expects a specific ImageFilterRegistry 
                // It Throws: 
                // Return value of Joomla\CMS\Image\Image::getServiceRegistry() 
                // must be an instance of Joomla\CMS\Image\ImageFilterRegistry, instance of Joomla\CMS\Customimage\ImageFilterRegistry returned
		$image->filter("brightness",$options);  

Appended the source code for the plugin imagefiltertest which could be placed in plugins/system/ and the library Customimage which could be placed in /libraries/src/

imagefiltertest.zip

@chmst chmst changed the base branch from 4.0-dev to 4.1-dev January 31, 2022 15:29
@chmst chmst removed the PR-4.0-dev label Jan 31, 2022
@HLeithner HLeithner changed the base branch from 4.1-dev to 4.2-dev June 27, 2022 13:08
@HLeithner HLeithner requested a review from laoneo as a code owner June 27, 2022 13:08
@HLeithner
Copy link
Member

This pull request has automatically rebased to 4.2-dev.

@joomla-bot
Copy link
Contributor

This pull requests has been automatically converted to the PSR-12 coding standard.

@rdeutz rdeutz changed the base branch from 4.2-dev to 4.3-dev October 21, 2022 11:33
@rdeutz
Copy link
Contributor

rdeutz commented Oct 21, 2022

@wilsonge what are we doing with this PR?

@rdeutz rdeutz added Maintainers Checked Used if the PR is conceptional useful PR-4.3-dev and removed PR-4.2-dev labels Oct 21, 2022
@HLeithner HLeithner removed the psr12 label Oct 23, 2022
@Hackwar Hackwar added the Feature label Apr 6, 2023
@HLeithner HLeithner changed the base branch from 4.3-dev to 5.0-dev May 8, 2023 15:03
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.0-dev. No new features will be merged into Joomla! 4.3 series. Joomla! 4.4 series is a bridge release to make migration from Joomla! 4 to 5 as smooth as possible.

@HLeithner HLeithner changed the base branch from 5.0-dev to 5.1-dev September 30, 2023 22:52
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.1-dev.

@rdeutz rdeutz self-assigned this Oct 24, 2023
@HLeithner HLeithner changed the base branch from 5.1-dev to 5.2-dev April 24, 2024 09:10
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.2-dev.

@HLeithner HLeithner changed the title [4.0] Rebuild the image filter to a registry allowing custom filters outside the main namespace [5.2] Rebuild the image filter to a registry allowing custom filters outside the main namespace Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants