EOL/eol_php_code — pull request #113 by hyanwong (2014-12-02 22:37:31+00:00)
Title: Store crop location in DB and use hard links if possible (2nd attempt)

As discussed with Jeremy Rice, this code implements several improvements to ContentManager.php, in particular in the grab_file() function:

1) When grab_file is called with a local file rather than a url, the code now tries to create a hard link to that file, instead of downloading a new copy (if it can't it falls back to downloading as before). This means when we do a custom crop of an image, we can keep the original download (which may be in a png, tiff, etc form, and so potentially a large file) without increasing disk usage. Previously, cropping an image resulted in losing the original downloaded file and retaining only an EOL-created jpg version.

2) When grab_file() is called on an image with a data_object_id given in the $options parameter, we store the height and width of the image in the image_sizes table in the EOL database (done by functions called within create_local_files_and_thumbnails()). I have checked that all calls to grab_file(xxx, "image") in the EOL codebase have been changed to pass in the data_object_id, apart from cases where we are grabbing thumbnails, or where we are testing the server (e.g. in test_create_load.php). Previously we did not store this image data.

3) When a custom crop is called (via crop_image()), the values are converted to percentages, then passed in to grab_file, which stores them in the image_sizes table. When grab_file is called on an image that already has a data_object_id, we take any previous crop values from the database and re-apply them, so that we get the same crops even when we reharvest. Previously, reharvesting caused all user-initiated crops to be lost.

4) Unit tests have been implemented for all these scenarios, in test_content_manager.php.

All this has involved a substantial rewrite of the code in ContentManager.php. I have tried to keep the logic exactly the same, and I have conducted limited testing (including running the unit tests). However, I do not have a full EOL setup on which to test this new code, so further testing should probably be done on the EoL development system. One particular case is where resources or datasets are in a tar/zip/gzip files and are unpacked by the grab_file routine. There is no unit test for this and I have commented that one should be added (there are also no unit tests of grab_file() for types "partner" and "dataset"). It might be a useful job for the new developers to check on the logic flow of my new code and add test cases.

There are a few places where I have kept the existing code but I think it should be changed or is redundant. I have marked these with a comment which includes 4 asterisks ****. The most major of these is a suggestion that translating the crop coordinates to percentages should be done by the Ruby front-end, since these calculations are dependent on features of the web page (in particular, the restriction of 540 pixel width for displaying the croppable image). It would both be easier and more logical to do web-page specific calculations in the RoR code than to pollute the php code with web-specific parameters as happens at the moment. As a stop-gap measure I have inserted a function crop_image_pct() which can be called by if the Ruby code is altered to produce x, y, and width parameters as percentages from the 580_360.jpg file.

There are also cases where I have implemented a "hack" to work around the fact that EoL does not store on a database the downloaded file extension or (in the case of images) details of any rotations that have been done to the original. There is a good argument that these should be kept (extension in the data_objects table, rotation details in the image_sizes table.

This is the second attempt at committing, with changes incorporated after discussion at https://github.com/EOL/eol_php_code/pull/111