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

add GaufretteFilesystemLoader #63

Merged
merged 6 commits into from
Apr 22, 2012
Merged

Conversation

havvg
Copy link
Contributor

@havvg havvg commented Apr 4, 2012

Example service definitions (using the FilesystemMap of KnpGaufretteBundle):

services:
    gaufrette.filesystem.profile_photos:
        class: Gaufrette\Filesystem
        factory_service: 'knp_gaufrette.filesystem_map'
        factory_method: 'get'
        arguments:
            - 'profile_photos'

    liip_imagine.data.loader.gaufrette.profile_photos:
        class: "%liip_imagine.data.loader.gaufrette.class%"
        arguments:
            - "@liip_imagine"
            - "@gaufrette.filesystem.profile_photos"
        tags:
            - { name: 'liip_imagine.data.loader', loader: 'gaufrette.profile_photos' }

Example filter set:

liip_imagine:
    filter_sets:
        profile_photo:
            data_loader: 'gaufrette.profile_photos'
            filters:
                thumbnail: { size: [300, 300], mode: outbound }

@stof
Copy link
Contributor

stof commented Apr 4, 2012

I'm wondering whether this loader is needed or if we should implement a StreamLoader able to load from any stream instead. In this case, we could use Gaufrette (as it provides a stream wrapper) but also other streams.

@lsmith77
Copy link
Contributor

@stof not sure what you mean exactly.

@havvg this PR definitely also needs an update to the README, though I guess first lets see what @stof was talking about.

@stof
Copy link
Contributor

stof commented Apr 11, 2012

@lsmith77 This PR adds a loader expecting the Gaufrette class to access files handled by Gaufrette. But it could use a far more generic solution if it was adding a loader able to load from a stream instead (any stream, not only the local filesystem). Gaufrette has a great feature: it implements a stream wrapper, meaning than any library expecting to open a stream would be able to open a file hidden behind Gaufrette (and so located anywhere) withoutt needing to know about Gaufrette. For instance, you can ask AsseticBundle to dump to your remote server handled by Gaufrette by using gaufrette://foobar/ as write_to path (there used to be an issue with S3 a while ago. Dunno if it has been fixed as I stopped stoffing Gaufrette a few months ago because of missing time)

@lsmith77
Copy link
Contributor

ok .. that sounds indeed like the better approach .. @havvg what do you think?

@havvg
Copy link
Contributor Author

havvg commented Apr 14, 2012

Hey there, sorry for the delay, I was on vacations.

The generalization sounds good to me, can't promise any ETA by now, but will look into it.

@havvg
Copy link
Contributor Author

havvg commented Apr 18, 2012

An example configuration to use the StreamLoader:

services:
    liip_imagine.data.loader.stream.profile_photos:
        class: "%liip_imagine.data.loader.stream.class%"
        arguments:
            - "@liip_imagine"
            - 'gaufrette://profile_photos/'
        tags:
            - { name: 'liip_imagine.data.loader', loader: 'stream.profile_photos' }

By adding some kind of factory, one could also inject an active stream context as third parameter.

* The error suppressing is solely to determ whether the file exists.
* file_exists() is not used as not all wrappers support stat() to actually check for existing resources.
*/
if (($this->context and @!fopen($name, 'r', null, $this->context)) || @!fopen($name, 'r')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use && instead of and

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, you should close the stream you just opened to avoid keeping an eventual lock

@lsmith77
Copy link
Contributor

can you add your example config to the README?

* @param string $wrapperPrefix
* @param \resource|null $context
*/
public function __construct(ImagineInterface $imagine, $wrapperPrefix, \resource $context = null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you really typehint a resource ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, no you can't, fixed.

The `Liip\ImagineBundle\Imagine\Data\Loader\StreamLoader` allows to read images from any stream registered
thus allowing you to serve your images from literally anywhere.

The example service definition shows how to use a stream wrapped by the [Gaufrette](https://github.com/KnpLabs/Gaufrette) filesystem abstraction layer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be good to clarify that this means one has to register the gaufrette stream wrapper for this to work.

lsmith77 added a commit that referenced this pull request Apr 22, 2012
@lsmith77 lsmith77 merged commit dfed61a into liip:master Apr 22, 2012
@neilferreira
Copy link

Hey guys,

I'm trying to use the Streamloder you've just setup, which looks to be pretty good based of what it should be capable of. What I'm getting stuck with at the moment is the following,

inside the Gaufrette readme example it shows that you've got to call 'StreamWrapper::register' manually to register the actual streamwrapper, at which point and where exactly do I need to register my actual stream? Do you have any real life examples of where you've used the StreamLoader for your profile images?

Where in your codebase do have you registered your profile_photos stream, and what type of filesystem are you using for it to dump the contents of that stream to so the StreamLoader can read from it?

Cheers.

@stof
Copy link
Contributor

stof commented May 2, 2012

I would register the Gaufrette wrapper in the boot method of a bundle so that it is done before the runtime logic

@neilferreira
Copy link

Hey,

I wasn't aware that method exists, but makes sense. I'm still eagler to know if @havvg has any further examples to share around.

Cheers

@lsmith77
Copy link
Contributor

lsmith77 commented May 2, 2012

either way .. we should expand the read me to cover this question

@havvg
Copy link
Contributor Author

havvg commented May 2, 2012

Hi there,

I created a gist how I solved this problem.
It's using dependency injection with tagged services to register the stream to be wrapped.

@neilferreira We are storing the images on Amazon S3, originals, cropped and cached (using AmazonS3Resolver from #66)

@neilferreira
Copy link

Hey,

Thanks for the gist, my next task after getting the StreamLoader working was to do exactly what you've done, storing originals and cropped copies of the images on AmazonS3, feel free to post some more gist's of the rest of your setup :)

Do you have functionality you've been using inside https://github.com/Ormigo/LiipImagineBundle ? Or have you merged all of your changes into the Liip Imagine bundle master via PRs.

Cheers

@havvg
Copy link
Contributor Author

havvg commented May 2, 2012

The fork only contains the #66 PR, no other diff beside that, we are on the respective feature branch.
The README of this PR contains how we set up the services.

The PR also adds a remove method to the CacheManager to be able to remove the respective files, it the user uploads a new version of his profile photo.

@skigun
Copy link

skigun commented Oct 12, 2012

Hi there,

I recently send a PR in a KnpGaufretteBundle KnpLabs/KnpGaufretteBundle#23
Now the stream wrapper can be registered and configured globally in KnpGaufretteBundle

@lsmith77
Copy link
Contributor

can you send a PR to make the necessary adjustments?

@KeKs0r
Copy link

KeKs0r commented Jul 3, 2013

I really would like to see a full example how this is used. I tried to create an streamwrapper in gaufrette for Amazon S3 and then use it with this bundle. The result for the filtered file is something like this:

http://localhost/tipp/web/app_dev.php/media/cache/profile_thumb_small/gaufrette://profile_fs/51d37d420eab4.png

I also setup the cacheresolver for S3: https://github.com/liip/LiipImagineBundle/blob/master/Resources/doc/cache-resolver/amazons3.md

Somehow I just dont get images from S3 working. (I would like to also store the cached images on S3)

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.

None yet

6 participants