Hi there,

I'm using the services 3 module to create a REST API. There's the option of either having XML format or JSON format returned.
These formats are defined as application/xml and application/json within the RESTServer.

Now, I'd like to have boost'd these REST requests but the current boost XML and JSON config doesn't allow these types.

Allowed types for boosting XML:

  • application/rss
  • text/xml
  • application/rss+xml

And for boosting JSON there's just the one type allowed:

  • text/javascript

So, the question I'm having now is whether services defines the wrong formats or you guys can add these two types into the conditions.

For clarification here's the code in question (boost.module - function _boost_ob_handler() - line 1367ff):

    if (stristr($types, 'text/javascript')) {
      if ($status == 200 & $GLOBALS['_boost_cache_this']) {
        if (BOOST_ASYNCHRONOUS_OUTPUT && !headers_sent()) {
          boost_async_opp($buffer, FALSE, 'text/javascript; charset=utf-8');
        }
        boost_cache_set($GLOBALS['_boost_path'], $decompressed_buffer, BOOST_JSON_EXTENSION);
      }
      elseif ($status == 404 || $status == 403) {
        $filename = boost_file_path($GLOBALS['_boost_path'], TRUE, BOOST_JSON_EXTENSION);
      }
    }
    elseif (stristr($types, 'application/rss') || stristr($types, 'text/xml') || stristr($types, 'application/rss+xml')) {
      if ($status == 200 && $GLOBALS['_boost_cache_this']) {
        if (BOOST_ASYNCHRONOUS_OUTPUT && !headers_sent()) {
          boost_async_opp($buffer, FALSE, 'text/xml; charset=utf-8');
        }
        boost_cache_set($GLOBALS['_boost_path'], $decompressed_buffer, BOOST_XML_EXTENSION);
      }
      elseif ($status == 404 || $status == 403) {
        $filename = boost_file_path($GLOBALS['_boost_path'], TRUE, BOOST_XML_EXTENSION);
      }
    }
CommentFileSizeAuthor
#23 boost-1219484-application-json-support-for-7x.1x.patch1.61 KBAnonymous (not verified)
#22 boost-types-for-services-1219484-22-D7.patch925 bytessteven.wichers
#11 boost-typesforservices-1219484-11.patch963 bytesjamix
#10 patch-1219484-rares.diff2.68 KBrares
#9 patch-1219484.diff2.38 KBrares
#1 boost-typesforservices-1219484-2-D6.patch738 bytessitiveni
#1 boost-typesforservices-1219484-2-D7.patch391 bytessitiveni
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sitiveni’s picture

sitiveni CreditAttribution: sitiveni commented
FileSize
boost-typesforservices-1219484-2-D7.patch391 bytes
boost-typesforservices-1219484-2-D6.patch738 bytes

Not having heard any objections I went forward and created the appropriate patches.
Noticed that D7 already contains application/xml as storage type variant, so added only application/json as variant for text/javascript.

sitiveni’s picture

sitiveni CreditAttribution: sitiveni commented
Status: Active » Needs review
MPeli’s picture

MPeli CreditAttribution: MPeli commented

I applied boost-typesforservices-1219484-2-D7.patch and json files are not cached. Do I have to modify something else?

Thank you.

bgm’s picture

bgm CreditAttribution: bgm commented
Title: Types application/xml and application/json not supported » Types application/xml and application/json not supported (support for services module)
Version: 6.x-1.18 » 7.x-1.x-dev
Status: Needs review » Active

Thanks for the patch. Committed to 6.x-1.x.

I have to admit I don't have a way to test it right now (well, without spending 1h configuring it), but the patch makes sense and low risk.

Moving this issue to 7.x-1.x for the patch to be applied there as well.

jwhat’s picture

jwhat CreditAttribution: jwhat commented

I just want to mention that I've been using a similar patch for a while but with 1 additional change. I have JSON feeds (created with the Services module) and without the following adjustment I would get the "Page cached by Boost" message at the bottom of my JSON feeds which caused them to break.

I added && $_boost['matched_header_info']['extension'] != 'json') below:

 // Add note to bottom of content if possible.
  if ($_boost['matched_header_info']['comment_start'] && $_boost['matched_header_info']['comment_end'] && $_boost['matched_header_info']['extension'] != 'json') { 
    $expire = $_boost['matched_header_info']['lifetime_max'];
    $cached_at = date('Y-m-d H:i:s', REQUEST_TIME);
    $expires_at = date('Y-m-d H:i:s', REQUEST_TIME + $expire);
    $note = "\n" . $_boost['matched_header_info']['comment_start'] . 'Page cached by Boost @ ' . $cached_at . ', expires @ ' . $expires_at . ', lifetime ' . format_interval($expire) . $_boost['matched_header_info']['comment_end'];
    $data .= $note;
  }

Do you have an alternative way of disabling this comment?

dayson’s picture

dayson CreditAttribution: dayson commented

Has this patch been applied and released in the recommended 7.x release? if not. when can we expect it?

bgm’s picture

bgm CreditAttribution: bgm commented

@ dayson: can you test the patch? If it works, mark this issue as "reviewed and tested".

Thanks

dayson’s picture

dayson CreditAttribution: dayson commented

@bgm Just tested it out. The patch does the job of allowing boost to be enabled with application/json responses. But as pointed out by @jwhat later that when caching application/json responses, it is essential to remove the "Page cached by Boost @ " message at the bottom of the response as it is invalid json. His solution to it works as well. So besides applying the submitted patch, you'll also have to apply the changes jwhat suggested.

rares’s picture

rares CreditAttribution: rares commented
Version: 7.x-1.x-dev » 6.x-1.x-dev
FileSize
patch-1219484.diff2.38 KB

On D6, I had to make several additional changes to make application/json caching to work, including:
- remove output of comments, since they are not supported in JSON at all
- added handling for application/json in the _boost_ob_handler() function.
I also noticed two other typos that likely made XML caching unfunctional.

This patch is against 6.x-dev. Feel free to port it to D7.

rares’s picture

rares CreditAttribution: rares commented
Status: Active » Needs review
FileSize
patch-1219484-rares.diff2.68 KB

re-uploading patch and setting review flag to that the testbot can see it

jamix’s picture

jamix CreditAttribution: jamix commented
Version: 6.x-1.x-dev » 7.x-1.x-dev
FileSize
boost-typesforservices-1219484-11.patch963 bytes

Consolidating patches from #1 and #5 an re-rolling against the latest 7.x-1.x-dev.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

Why change the comments to javascript / css when the output is targeted to not be json ?

$_boost['matched_header_info']['extension'] != 'json'

jamix’s picture

jamix CreditAttribution: jamix commented

The idea is not to output the Boost comment if the Content-Type is "application/json" because comments break JSON syntax.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

Yes I understand that, but my reading of the conditional 

&& $_boost['matched_header_info']['extension'] != 'json'

suggests that for html etc... that the comments would be changed to /* and */

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

I would have thought that the code below would be more effective and be more explanatory.

  // Add note to bottom of content if possible.
  if ($_boost['matched_header_info']['comment_start'] && $_boost['matched_header_info']['comment_end']) {
// 2 NEW LINES
    // no comments allowed in json data
    if ( $_boost['matched_header_info']['extension'] != 'json' ) {
    $expire = $_boost['matched_header_info']['lifetime_max'];
    $cached_at = date('Y-m-d H:i:s', REQUEST_TIME);
    $expires_at = date('Y-m-d H:i:s', REQUEST_TIME + $expire);
    $note = "\n" . $_boost['matched_header_info']['comment_start'] . 'Page cached by Boost @ ' . $cached_at . ', expires @ ' . $expires_at . ', lifetime ' . format_interval($expire) . $_boost['matched_header_info']['comment_end'];
    $data .= $note;
// 1 NEW LINE
    }
  }
jamix’s picture

jamix CreditAttribution: jamix commented

Adding the extra nested if equals to &&'ing its condition to the outer if, so the logic stays the same. Or are we talking cosmetics/coding style here?

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

My knocked up little version does adhere to the coding standards slightly better (probably could do with some extra indents after the if), but more importantly it doesn't modify the comment structure or variants array, so is less likely to produce unexpected results.

jamix’s picture

jamix CreditAttribution: jamix commented

The patch in #11 consists of two parts. First, we modify the variants array for caching to actually work for the "application/json" content type. Second, we ensure that the Boost comment doesn't get output for this content type. Comment structure isn't modified at all.

There must be some misunderstanding going on because I fail to understand what's wrong with the patch.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented 

function boost_boost_storage_types() {
  $types = array();
..... SNIP .....

  $types['text/xml'] = array(
    'title'           => t('XML'),
    'description'     => t('XML output, usually a feed'),
    'extension'       => 'xml',
    'enabled'         => FALSE,
    'gzip'            => TRUE,
    'lifetime_max'    => 3600,
    'lifetime_min'    => 0,
    'comment_start'  => '<!-- ',
    'comment_end'    => ' -->',
    'variants'        => array(
      'application/xml',
      'application/rss',
      'application/rss+xml',
    ),
  );
  $types['text/javascript'] = array(
    'title'           => t('AJAX/JSON'),
    'description'     => t('JSON output, usually a response to a AJAX request'),
    'extension'       => 'json',
    'enabled'         => FALSE,
    'gzip'            => TRUE,
    'lifetime_max'    => 3600,
    'lifetime_min'    => 0,
    'comment_start'  => '/* ',
    'comment_end'    => ' */',
    'variants'        => array(),
  );
  return $types;
}

What I don't understand is why you think that 7.x doesn't serve json or have the type defined as in the code above in boost.module. Though really the comments should just be removed for JSON in the above, which would probably turn off the output without needing to alter boost exit. So the patch would be 


//    'comment_start'  => '/* ',
//    'comment_end'    => ' */',
jamix’s picture

jamix CreditAttribution: jamix commented

What I don't understand is why you think that 7.x doesn't serve json or have the type defined as in the code above in boost.module.

When JSON is output by Drupal with the Content-Type of "application/json" instead of "text/javascript" (with "application/json" being the proper Content-Type for JSON, BTW), Boost fails to cache the response. Please see sitiveni's original message in the thread and just try it yourself.

The patch in #11 allows one to enable caching for the "application/json" Content-Type as well as to suppress Boost comment for this particular case.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

I have looked at the original post, and it would make more sense to add an additional type to the function boost_boost_storage_types, which is optionally selected in the boost menu options (without comments and correcting the current output), than to alter boost_exit.

That then extends functionality across any applications that output the wrong headers, rather than adding a conditional in the wrong place (IMHO).

steven.wichers’s picture

steven.wichers CreditAttribution: steven.wichers commented
FileSize
boost-types-for-services-1219484-22-D7.patch925 bytes

Rerolled patch #11 against 7.x-1.0-beta2

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Assigned: Unassigned »
FileSize
boost-1219484-application-json-support-for-7x.1x.patch1.61 KB

This patch adds support for two extra types of javascript (rightly or wrongly if the headers are set for JSON request or even the jquery.getScript() function).

  • text/javascript assigned to a .js extension == boost comments in output
  • application/javascript to a json extension == no boost comments in output
  • application/json to a json extension == no boost comments in output

leaving mod_mime (etc.. for other servers and the existing drupal rewrite rules/ caching) to deal with application/javascript with a .js extension.

jeff h’s picture

jeff h CreditAttribution: jeff h commented

This looks great — did it ever get rolled in?

cwillinx’s picture

cwillinx CreditAttribution: cwillinx commented

Cached json files not cleared on expire

All this works very well for me, but the cached json files don't get cleared when listed via the expire module.

I think that the reason for this is here:
in function boost_expire_cache($urls) {
there is called boost_get_header_info() which is being prefilled:

function boost_get_header_info() {
  $headers = drupal_get_http_header();
  $status = '200 OK';
  $status_number = '200';
  $content_type = 'text/html; charset=utf-8';
  $content_type_basic = 'text/html';
  $charset = 'utf-8';

  foreach ($headers as $name_lower => $value) {

Since drupal_get_http_header() cannot give back a result when there is no header (it only works for the current generated page, the documentation says) all types (html, json, xml) will be misdetected as html.

Because of this boost is always generating a filename with an html extention, which of course cannot delete the file originally and correctly saved with an json extention.

So in my opinion, the method of finding the correct filename via drupal_get_http_header() is completely wrong.
Json never gets erased that way!

What could be done? Listing the directory and parsing filenames for JSON extentions??
Any better ideas?

Any help is appreciated!

garbo’s picture

garbo CreditAttribution: garbo commented

The patch of #23 seems to work for me. My JSON pages (generated with drupal_json_output()) are being cached now.

I would very much like to see this committed!

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented

Patch #23 worked great for us using a ms 'WIMP' stack, had to add a mime type for application/json and we had to add a rewrite rule from here (for IIS only)

except in the rewriteRule.config (iis only) change CONTENT_TYPE from text/javascript to application/json

(we've not yet switched to apache /LAMP but are moving in that direction)

I imagine most everyone else above is using Apache.

Thanks a bunch for the patch! Our ajax requests are now amazingly fast taking less than a millisecond .

ruloweb’s picture

ruloweb CreditAttribution: ruloweb commented
Status: Needs review » Reviewed & tested by the community

Patch #23 works like a charm! I think we can move it to Reviewed.

Thanks!

  • bgm committed 94fa47e on 7.x-1.x authored by Philip_Clarke 
    Issue #1219484 by sitiveni, rares, Philip_Clarke, jamix, steven.wichers...
bgm’s picture

bgm CreditAttribution: bgm commented
Status: Reviewed & tested by the community » Fixed

Thanks for the patch and testing/feedback. Merged to 7.x-1.x.

I see that there were concerns noted in response to #23, but that nonetheless it works better with the patch than without. If there are more specific issues, please open a new ticket and link it to this one.

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented

@bgm, thanks for the commit, we've been using it for a while, as for comment#25 , we implemented our own hook_expire hook that deals with the .json , that's out of scope for this fix, so great work everyone and thanks again!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.