Bug 569177 - Add support for eTag for WebServices

r/a=glob

7 files changed, 131 insertions, 18 deletions

diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm
index dacd900a0..991bfc05d 100644
--- a/Bugzilla/CGI.pm
+++ b/Bugzilla/CGI.pm
@@ -236,11 +236,11 @@ sub check_etag {
         $possible_etag =~ s/^\"//g;
         $possible_etag =~ s/\"$//g;
         if ($possible_etag eq $valid_etag or $possible_etag eq '*') {
-            print $self->header(-ETag => $possible_etag,
-                                -status => '304 Not Modified');
-            exit;
+            return 1;
         }
     }
+
+    return 0;
 }
 
 # Have to add the cookies in.
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index c639084ef..58405c2d0 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -349,6 +349,18 @@ sub get {
         push(@bugs, $self->_bug_to_hash($bug, $params));
     }
 
+    # Set the ETag before inserting the update tokens
+    # since the tokens will always be unique even if
+    # the data has not changed.
+    $self->bz_etag(\@bugs);
+
+    if (Bugzilla->user->id) {
+        foreach my $bug (@bugs) {
+            my $token = issue_hash_token([$bug->{'id'}, $bug->{'last_change_time'}]);
+            $bug->{'update_token'} = $self->type('string', $token);
+        }
+    }
+
     return { bugs => \@bugs, faults => \@faults };
 }
 
@@ -986,11 +998,6 @@ sub _bug_to_hash {
         }
     }
 
-    if (Bugzilla->user->id) {
-        my $token = issue_hash_token([$bug->id, $bug->delta_ts]);
-        $item{'update_token'} = $self->type('string', $token);
-    }
-
     # The "accessible" bits go here because they have long names and it
     # makes the code look nicer to separate them out.
     $item{'is_cc_accessible'} = $self->type('boolean', 
diff --git a/Bugzilla/WebService/Server.pm b/Bugzilla/WebService/Server.pm
index 02226284c..5ffd965a5 100644
--- a/Bugzilla/WebService/Server.pm
+++ b/Bugzilla/WebService/Server.pm
@@ -14,6 +14,9 @@ use Bugzilla::Error;
 use Bugzilla::Util qw(datetime_from);
 
 use Scalar::Util qw(blessed);
+use Digest::MD5 qw(md5_base64);
+
+use Storable qw(freeze);
 
 sub handle_login {
     my ($self, $class, $method, $full_method) = @_;
@@ -29,7 +32,7 @@ sub handle_login {
 
 sub datetime_format_inbound {
     my ($self, $time) = @_;
-    
+
     my $converted = datetime_from($time, Bugzilla->local_timezone);
     if (!defined $converted) {
         ThrowUserError('illegal_date', { date => $time });
@@ -55,8 +58,63 @@ sub datetime_format_outbound {
     return $time->iso8601();
 }
 
+# ETag support
+sub bz_etag {
+    my ($self, $data) = @_;
+    my $cache = Bugzilla->request_cache;
+    if (defined $data) {
+        # Serialize the data if passed a reference
+        local $Storable::canonical = 1;
+        $data = freeze($data) if ref $data;
+
+        # Wide characters cause md5_base64() to die.
+        utf8::encode($data) if utf8::is_utf8($data);
+
+        # Append content_type to the end of the data
+        # string as we want the etag to be unique to
+        # the content_type. We do not need this for
+        # XMLRPC as text/xml is always returned.
+        if (blessed($self) && $self->can('content_type')) {
+            $data .= $self->content_type if $self->content_type;
+        }
+
+        $cache->{'bz_etag'} = md5_base64($data);
+    }
+    return $cache->{'bz_etag'};
+}
+
 1;
 
+=head1 NAME
+
+Bugzilla::WebService::Server - Base server class for the WebService API
+
+=head1 DESCRIPTION
+
+Bugzilla::WebService::Server is the base class for the individual WebService API
+servers such as XMLRPC, JSONRPC, and REST. You never actually create a
+Bugzilla::WebService::Server directly, you only make subclasses of it.
+
+=head1 FUNCTIONS
+
+=over
+
+=item C<bz_etag>
+
+This function is used to store an ETag value that will be used when returning
+the data by the different API server modules such as XMLRPC, or REST. The individual
+webservice methods can also set the value earlier in the process if needed such as
+before a unique update token is added. If a value is not set earlier, an etag will
+automatically be created using the returned data except in some cases when an error
+has occurred.
+
+=back
+
+=head1 SEE ALSO
+
+L<Bugzilla::WebService::Server::XMLRPC|XMLRPC>, L<Bugzilla::WebService::Server::JSONRPC|JSONRPC>,
+and L<Bugzilla::WebService::Server::REST|REST>.
+
 =head1 B<Methods in need of POD>
 
 =over
diff --git a/Bugzilla/WebService/Server/JSONRPC.pm b/Bugzilla/WebService/Server/JSONRPC.pm
index d4573fd19..e7b3fe7e7 100644
--- a/Bugzilla/WebService/Server/JSONRPC.pm
+++ b/Bugzilla/WebService/Server/JSONRPC.pm
@@ -75,12 +75,12 @@ sub response_header {
 
 sub response {
     my ($self, $response) = @_;
+    my $cgi = $self->cgi;
 
     # Implement JSONP.
     if (my $callback = $self->_bz_callback) {
         my $content = $response->content;
         $response->content("$callback($content)");
-
     }
 
     # Use $cgi->header properly instead of just printing text directly.
@@ -95,9 +95,18 @@ sub response {
             push(@header_args, "-$name", $value);
         }
     }
-    my $cgi = $self->cgi;
-    print $cgi->header(-status => $response->code, @header_args);
-    print $response->content;
+
+    # ETag support
+    my $etag = $self->bz_etag;
+    if ($etag && $cgi->check_etag($etag)) {
+        push(@header_args, "-ETag", $etag);
+        print $cgi->header(-status => '304 Not Modified', @header_args);
+    }
+    else {
+        push(@header_args, "-ETag", $etag) if $etag;
+        print $cgi->header(-status => $response->code, @header_args);
+        print $response->content;
+    }
 }
 
 # The JSON-RPC 1.1 GET specification is not so great--you can't specify
@@ -257,7 +266,17 @@ sub _handle {
     my $self = shift;
     my ($obj) = @_;
     $self->{_bz_request_id} = $obj->{id};
-    return $self->SUPER::_handle(@_);
+
+    my $result = $self->SUPER::_handle(@_);
+
+    # Set the ETag if not already set in the webservice methods.
+    my $etag = $self->bz_etag;
+    if (!$etag && ref $result) {
+        my $data = $self->json->decode($result)->{'result'};
+        $self->bz_etag($data);
+    }
+
+    return $result;
 }
 
 # Make all error messages returned by JSON::RPC go into the 100000
diff --git a/Bugzilla/WebService/Server/REST.pm b/Bugzilla/WebService/Server/REST.pm
index 00c71110f..8d3aa481c 100644
--- a/Bugzilla/WebService/Server/REST.pm
+++ b/Bugzilla/WebService/Server/REST.pm
@@ -125,6 +125,10 @@ sub response {
     # Access Control
     $response->header("Access-Control-Allow-Origin", "*");
 
+    # ETag support
+    my $etag = $self->bz_etag;
+    $self->bz_etag($result) if !$etag;
+
     # If accessing through web browser, then display in readable format
     if ($self->content_type eq 'text/html') {
         $result = $self->json->pretty->canonical->encode($result);
diff --git a/Bugzilla/WebService/Server/XMLRPC.pm b/Bugzilla/WebService/Server/XMLRPC.pm
index 93c9b4cdc..40c66a8f9 100644
--- a/Bugzilla/WebService/Server/XMLRPC.pm
+++ b/Bugzilla/WebService/Server/XMLRPC.pm
@@ -21,8 +21,8 @@ if ($ENV{MOD_PERL}) {
 use Bugzilla::WebService::Constants;
 use Bugzilla::Util;
 
-# Allow WebService methods to call XMLRPC::Lite's type method directly
 BEGIN {
+    # Allow WebService methods to call XMLRPC::Lite's type method directly
     *Bugzilla::WebService::type = sub {
         my ($self, $type, $value) = @_;
         if ($type eq 'dateTime') {
@@ -39,6 +39,11 @@ BEGIN {
         }
         return XMLRPC::Data->type($type)->value($value);
     };
+
+    # Add support for ETags into XMLRPC WebServices
+    *Bugzilla::WebService::bz_etag = sub {
+        return Bugzilla::WebService::Server->bz_etag($_[1]);
+    };
 }
 
 sub initialize {
@@ -52,22 +57,38 @@ sub initialize {
 
 sub make_response {
     my $self = shift;
+    my $cgi = Bugzilla->cgi;
 
     $self->SUPER::make_response(@_);
 
     # XMLRPC::Transport::HTTP::CGI doesn't know about Bugzilla carrying around
     # its cookies in Bugzilla::CGI, so we need to copy them over.
-    foreach my $cookie (@{Bugzilla->cgi->{'Bugzilla_cookie_list'}}) {
+    foreach my $cookie (@{$cgi->{'Bugzilla_cookie_list'}}) {
         $self->response->headers->push_header('Set-Cookie', $cookie);
     }
 
     # Copy across security related headers from Bugzilla::CGI
-    foreach my $header (split(/[\r\n]+/, Bugzilla->cgi->header)) {
+    foreach my $header (split(/[\r\n]+/, $cgi->header)) {
         my ($name, $value) = $header =~ /^([^:]+): (.*)/;
         if (!$self->response->headers->header($name)) {
            $self->response->headers->header($name => $value);
         }
     }
+
+    # ETag support
+    my $etag = $self->bz_etag;
+    if (!$etag) {
+        my $data = $self->response->as_string;
+        $etag = $self->bz_etag($data);
+    }
+
+    if ($etag && $cgi->check_etag($etag)) {
+        $self->response->headers->push_header('ETag', $etag);
+        $self->response->headers->push_header('status', '304 Not Modified');
+    }
+    elsif ($etag) {
+        $self->response->headers->push_header('ETag', $etag);
+    }
 }
 
 sub handle_login {
diff --git a/config.cgi b/config.cgi
index 4c0881a11..02f954726 100755
--- a/config.cgi
+++ b/config.cgi
@@ -142,7 +142,11 @@ sub display_data {
     utf8::encode($digest_data) if utf8::is_utf8($digest_data);
     my $digest = md5_base64($digest_data);
 
-    $cgi->check_etag($digest);
+    if ($cgi->check_etag($digest)) {
+        print $cgi->header(-ETag   => $digest,
+                           -status => '304 Not Modified');
+        exit;
+    }
 
     print $cgi->header (-ETag => $digest,
                         -type => $format->{'ctype'});