TinyPNG.com integration

Here is a version without the certs and which sets CURLOPT_HEADER => true as is in tinypng API docs.

1. +++ b/services/tinypng.inc
   @@ -0,0 +1,143 @@
   +    '#markup' => '<p>TinyPNG uses smart lossy compression techniques to reduce the file size of your PNG files. By selectively decreasing the number of colors in the image, fewer bytes are required to store the data. The effect is nearly invisible but it makes a very large difference in file size!</p><p>TinyPNG compression ONLY APPLIES TO PNG files. If your image is uploaded in other format, TinyPNG will not process the image and the default Image Toolkit processing will apply.</p><p>If you want to process existing PNG files, remove their image styles with the command <pre>drush image-flush --all</pre> and they will be recreated using TinyPNG the 1st time they are called.</p><p>Free licenses has a limit of conversions per user and per month. Please note that each time an image is uploaded to your Drupal site, an automatic request to TinyPNG is performed for EACH ONE of the Image Styles EFFECTIVELY USED for the image. This is the desired behavior, but it may quickly drain your monthly quota if many images are uploaded and many Image Styles are used.</p>'
   

   You should wrap the text itself with t function.

2. +++ b/services/tinypng.inc
   @@ -0,0 +1,143 @@
   +    '#element_validate' => array('imageapi_optimize_tinypng_validate_api_key')
   ...
   +    '#default_value' => $settings['debug_mode_tinypng']
   

   A comma should be added after the line.

3. +++ b/services/tinypng.inc
   @@ -0,0 +1,143 @@
   +  // Check if you have curl loaded
   ...
   +  // Check if json is available
   ...
   +  // Make the request
   

   After comments should be a ., ? or !.

4. +++ b/services/tinypng.inc
   @@ -0,0 +1,143 @@
   +    /* Compression was successful, retrieve output */
   

   For one line comments you should use // instead of /* */

Because of CURLOPT_HEADER set to TRUE, headers are in output. In that case, $response can not be decode. CURLOPT_HEADER must be set to FALSE.

Works fine after this little tweak.

Some whitespace cleanup

Refactored patch to work with drush patch file.

Typo in last patch.

I tested this patch with a 10MB jpeg and 8MB png and it worked great with no issues.

Patch #11 works great. Awesome feature addon! Thank you!

There's some bugs in the patch posted in #11 that leads to fatal errors. I fixed that in this new patch.

I'm using the patch in #14. Works great. Note that this works for jpgs as well as pngs.

Thanks for everyone's work on this, and the reviews.

I'm setting back to needs work, and raising the following points for discussion:

1. +++ b/services/tinypng.inc
   @@ -0,0 +1,137 @@
   +    '#title' => t('Enter your API key'),
   

   Rather than 'Enter your API key' this should be 'API key' or 'TinyPNG API key' if it's not clear from context.

2. +++ b/services/tinypng.inc
   @@ -0,0 +1,137 @@
   +    '#title' => t('<b>Enable debug mode</b>: show statistics (size reduction) from successful requests.'),
   

   I think this string should be split into two, and the '#description' property of the form element used.
   We can then also remove the 'b' tags.

3. +++ b/services/tinypng.inc
   @@ -0,0 +1,137 @@
   +    form_set_error($element['#parents'][0], t('TinyPNG API key must have 32-char length.'));
   

   Seems a bit odd to not just use form_error($element, '...'); is this done for a reason that I'm missing?

4. +++ b/services/tinypng.inc
   @@ -0,0 +1,137 @@
   +  if ($image->info['mime_type'] != 'image/png' && $image->info['mime_type'] != 'image/jpeg') {
   

   This is fine, but may be clearer if we have:
   !in_array($image->info['mime_type'], array('image/png', 'image/jpeg'))

   Is that clearer? Discuss!

5. +++ b/services/tinypng.inc
   @@ -0,0 +1,137 @@
   +  if (!function_exists('curl_init')) {
   

   Is there a reason for using cURL here, and not using drupal_http_request.

   Although drupal_http_request is a mess, I feel like we should use the provided API, which has a way of being swapped out and tweaked if needed, rather than forcing people to have cURL and hack the module if they need to make tweaks, like certificate bundles etc.

6. +++ b/services/tinypng.inc
   @@ -0,0 +1,137 @@
   +  if (!function_exists('json_decode')) {
   

   Drupal 7's requirements list the json extension, so while this is a nice to have a check here too, I don't think we should bother checking, we should just assume we have it.

   Also, we should probably use drupal_json_decode because sometimes different PHP versions do return different things from json_decode.

So sorry, there are some big things in there, but all up for discussion and further review.

I rolled all the changes suggested in #16 into this patch. I agree with all of them.

@thepractice thanks for the updated patch! Upon review, I've found a few more things :)
I don't have time to apply, tweak and commit, so have gone for the review for now, so that someone else could at least move this issue along:

1. +++ b/services/tinypng.inc
   @@ -0,0 +1,116 @@
   +    '#description' => t('Show statistics (size reduction) from successful requests.'),
   

   We should mention where the statistics will be displayed (and below I mention changing it to the watchdog).

2. +++ b/services/tinypng.inc
   @@ -0,0 +1,116 @@
   +    return FALSE;
   +  }
   +  return TRUE;
   

   I don't think we need to return anything here.

3. +++ b/services/tinypng.inc
   @@ -0,0 +1,116 @@
   +        drupal_set_message(t('Processed image received from TinyPNG.com could not be saved to disk on @dir. Please review the directory permissions'), array('@dir' => $filepath), 'warning');
   +        return FALSE;
   +      }
   +
   +      if ($settings['debug_mode_tinypng']) {
   +        drupal_set_message(t('Sucessful request to TinyPNG.com for @file', array('@file' => $dst)), 'status');
   +        drupal_set_message(t('Initial file size = @input bytes.<br>Final file size = @output bytes.<br>Compression ratio = @ratio.', array('@input' => $json['input']['size'], '@output' => $json['output']['size'], '@ratio' => $json['output']['ratio'])), 'status');
   +      }
   +      return TRUE;
   +    }
   +  }
   +
   +  // Something went wrong :(
   +  drupal_set_message(t('TinyPNG.com could not process your request for @file. Error code = @error - @msg', array('@file' => $filepath, '@error' => $json['error'], '@msg' => $json['message'])), 'warning');
   

   I'm not sure this should be a drupal_set_message, the end users of the site don't need to see this message.

   Can we change all these to watchdog messages please?

   The debug pair, can become a single watchdog message.

I've updated the patch from #17 to address the comments in #18.

1. We state that statistics will be displayed in log messages with a link to /admin/reports/dblog.

2. There is no need to return anything so I've removed the return statements.

3. I converted all the drupal messages to watchdog messages and fixed the first message so the t() paragraph marks close around the array of variables.

Thanks so much for the patch and the patience.

Fixed!