Mailing List Archive: Update alternate image with find_or_create

Update alternate image with find_or_create_alternate

May 23, 2011, 3:57 AM

Post #1 of 3 (2540 views)

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.

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?

---

Should it be activated by a configuration directive, or is the
'original_md5' field requirement sufficient?

--

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.

Regards,

Mike

Re: Update alternate image with find_or_create_alternate [ In reply to ]

david at kineticode

May 24, 2011, 9:57 AM

Post #2 of 3 (2411 views)

Permalink

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.

Great sounding work, though.

> 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'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.

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.

Oh yeah, and it'll need some tests. :-)

Great stuff, really appreciated.

Best,

David

Re: Update alternate image with find_or_create_alternate [ In reply to ]

catalyst at mikeraynham

May 24, 2011, 10:58 AM

Post #3 of 3 (2431 views)

Permalink

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
>