catalyst at mikeraynham
May 24, 2011, 10:58 AM
Post #3 of 3
(896 views)
Permalink
|
|
Re: Update alternate image with find_or_create_alternate
[In reply to]
|
|
On 24/05/11 17:57, David E. Wheeler wrote:
> On May 23, 2011, at 3:57 AM, Mike Raynham wrote:
>
>> I'd like find_or_create_alternate() to update the alternate image if the original image is updated. I've come up with a solution that I think is okay - my limited testing has shown it to work.
>
> Awesome, this has been much requested.
>
>> I'd like to check if others think it is a sane solution - I've still got a lot to learn about Bricolage, so there's a good chance that I've missed something.
>>
>> Description:
>>
>> The solution requires that the alternate image element type (as specified in the 'et_key_name' parameter) has an additional custom field:
>>
>> Key name: original_md5
>> Widet type: Text Box
>>
>> When find_or_create_alternate() is called, it checks for the existence of the 'original_md5' field in the alternate image. If it is not found, processing continues as it did previously.
>>
>> If 'original_md5' is present:
>>
>> When find_or_create_alternate() is first called, it generates an MD5 digest of the original image, and stores it in the 'original_md5' field of the alternate image.
>>
>> On calls to find_or_create_alternate() it checks the value stored in 'original_md5' against the actual MD5 of the original image.
>>
>> If the values are different, the original image is deemed to have changed. The existing alternate image is deleted and replaced with a new alternate image. The new alternate image 'original_md5' field is updated accordingly.
>>
>> If the values are the same, the original image is deemed to be the same as the alternate image. In this case, the alternate image is returned unaltered.
>>
>> Questions:
>>
>> Does this method make sense?
>
> It does, although I think I'd like to see something more permanent as part of the media class, rather than a magic field that will show up in the UI.
That sounds like a good plan. I'll look into it.
>
> Great sounding work, though.
Thanks.
>
>> Should it be activated by a configuration directive, or is the 'original_md5' field requirement sufficient?
>
> I think no magic field and activated by a configuration directive. Or, perhaps, by an additional argument to find_or_create_alternate(). Directive probably better, though.
>
> Unless no one objects and we can just turn it on permanently. I think that would be preferred by people, as it's less surprising than the current state. Maybe have a directive but enabled by default? I think that might be the best choice
I think a directive would be a good idea, just in case some are using
the existing mechanism in weird and wonderful ways.
>
>> I'm calling the following private methods to delete the alternate image:
>>
>> $alt->_delete_instance;
>> $alt->_delete_media;
>> $alt->_delete_file;
>>
>> Is there a better way to do this (without calling private methods)?
>>
>> ---
>>
>> I've created a new public method:
>>
>> Bric::Biz::Asset::Business::Media->get_md5()
>>
>> It uses Digest::MD5 to generate a 32 character hex MD5 digest of the media file. Is that okay? I thought it might be useful from within templates for images and other media types.
>
> Yeah, that's awesome. I wonder if maybe it should be stored. That is, add it as an actual stored database attribute of Media. Then you don't need the magic field, either.
Yes, I like that idea, it's a cleaner solution. I need to study the
schema some more to fully understand the media and media_instance
tables, but off the top of my head, I think two new DB columns would be
required (I'm not sure in which table):
* One for the actual digest of the media file. This would be populated
when the media file is uploaded and returned by the public `get_md5()`
method.
* One to store the digest of the alternate image parent (the value
currently held by my original_md5 magic field). This would be populated
by `find_or_create_alternate()`, and returned by a private method -
e.g., `_get_alternate_md5()`.
> To do that, you'd need to modify sql/(Pg|mysql)/Bric/Biz/Asset/Business/Media.(sql|val), and then add a migration script to inst/upgrade/2.1.0/. The version_columns.pl script there should be a decent example, although inst/upgrade/1.11.0/add_field_type_occurrence.pl might be clearer. You'll then need to iterate over all existing media to insert a value for the new field.
The magic field is sounding like a good idea again... :-)
> Oh yeah, and it'll need some tests. :-)
I've started reading through the existing tests, and am working on
setting up a suitable development environment. Any tips on the setup
for running the tests would be appreciated.
> Great stuff, really appreciated.
>
> Best,
>
> David
>
|
