[ARVADOS] updated: 123ce64a39849224481a67cc400a04c5022b639f

git at public.curoverse.com git at public.curoverse.com
Wed May 21 15:25:03 EDT 2014


Summary of changes:
 .../app/controllers/application_controller.rb      | 21 +-----
 .../app/controllers/collections_controller.rb      | 48 +++++++------
 .../app/views/collections/_show_files.html.erb     |  2 +-
 .../app/views/collections/show_file_links.html.erb | 82 ++++++++++++++++++++++
 .../app/views/layouts/application.html.erb         |  1 +
 apps/workbench/config/routes.rb                    |  3 +
 apps/workbench/public/robots.txt                   |  2 -
 .../test/functional/collections_controller_test.rb | 34 +++------
 .../workbench/test/integration/collections_test.rb | 27 ++++++-
 apps/workbench/test/integration_helper.rb          |  5 ++
 apps/workbench/test/test_helper.rb                 |  1 +
 .../api/app/models/api_client_authorization.rb     |  2 +-
 .../test/fixtures/api_client_authorizations.yml    |  7 ++
 13 files changed, 162 insertions(+), 73 deletions(-)
 create mode 100644 apps/workbench/app/views/collections/show_file_links.html.erb

       via  123ce64a39849224481a67cc400a04c5022b639f (commit)
       via  52056c365c8936e34bc6b22a01bcacb556737914 (commit)
       via  83277602806099428b859b88745ad85836cd6ee4 (commit)
       via  a2c261dada5b47077c7930a2133bb000631fab66 (commit)
       via  dffa580d39b977746f6950b835b78b949a862c9d (commit)
       via  1924b28c8f715bc08752fb219d1fd8145ccfe84f (commit)
       via  0bec8ec8a130cb3af013b6097cf322938fce5671 (commit)
       via  fa6b0b65fb71b66e36f982f4f25e9751672d0834 (commit)
       via  661b2bcd6269fae643fd7d0c46649715716959eb (commit)
       via  f876a32276a73d89ab695e5954c9ea3816b38db9 (commit)
      from  2dc7f15f81ea7f460114482614d8ec5814c36fbf (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 123ce64a39849224481a67cc400a04c5022b639f
Merge: 2dc7f15 52056c3
Author: Brett Smith <brett at curoverse.com>
Date:   Wed May 21 15:25:20 2014 -0400

    Merge branch '2764-wget-collections'
    
    Closes #2764, #2774, #2830.


commit 52056c365c8936e34bc6b22a01bcacb556737914
Author: Brett Smith <brett at curoverse.com>
Date:   Wed May 21 11:30:44 2014 -0400

    2764: UI overhaul to the Collection download page.
    
    Refs #2764, based on feedback.

diff --git a/apps/workbench/app/views/collections/show_file_links.html.erb b/apps/workbench/app/views/collections/show_file_links.html.erb
index 9d61a6a..de012c7 100644
--- a/apps/workbench/app/views/collections/show_file_links.html.erb
+++ b/apps/workbench/app/views/collections/show_file_links.html.erb
@@ -1,36 +1,82 @@
 <!DOCTYPE html>
 <html>
+<% coll_name = (@object.name =~ /\S/) ? @object.name : "Collection #{@object.uuid}" %>
+<% link_opts = {controller: 'collections', action: 'show_file',
+                uuid: @object.uuid, reader_token: params[:reader_token]} %>
 <head>
   <meta charset="utf-8">
   <title>
-    <% if content_for? :page_title %>
-    <%= yield :page_title %> / <%= Rails.configuration.site_name %>
-    <% else %>
-    <%= Rails.configuration.site_name %>
-    <% end %>
+    <%= coll_name %> / <%= Rails.configuration.site_name %>
   </title>
   <meta name="description" content="">
   <meta name="author" content="">
   <meta name="robots" content="NOINDEX">
+  <style type="text/css">
+body {
+  margin: 1.5em;
+}
+pre {
+  background-color: #D9EDF7;
+  border-radius: .25em;
+  padding: .75em;
+  overflow: auto;
+}
+.footer {
+  font-size: 82%;
+}
+.footer h2 {
+  font-size: 1.2em;
+}
+  </style>
 </head>
 <body>
-<% content_for :page_title do %>
-  <%= (@object.respond_to?(:properties) ? @object.properties[:page_title] : nil) ||
-        @object.friendly_link_name %>
-<% end %>
 
-<% if @object.andand.files.andand.any? %>
-  <% link_opts = {controller: 'collections', action: 'show_file',
-                  uuid: @object.uuid, reader_token: params[:reader_token]} %>
-  <ul>
-  <% @object.files.map { |spec|
-       CollectionsHelper::file_path(spec)
-     }.each do |path| %>
-    <li><%= link_to(path, link_opts.merge(file: path)) %></li>
+<h1><%= coll_name %></h1>
+
+<p>This collection of data files is being shared with you through
+Arvados.  You can download individual files listed below.  To download
+the entire collection with wget, try:</p>
+
+<pre>$ wget --mirror --no-parent --no-host --cut-dirs=3 <%=
+         url_for(link_opts.merge(action: 'show_file_links', only_path: false))
+       %></pre>
+
+<h2>File Listing</h2>
+
+<% if @object.andand.files_tree.andand.any? %>
+  <ul id="collection_files" class="collection_files">
+  <% dirstack = [@object.files_tree.first.first] %>
+  <% @object.files_tree.each_with_index do |(dirname, filename, size), index| %>
+    <% file_path = CollectionsHelper::file_path([dirname, filename]) %>
+    <% while dirstack.any? and (dirstack.last != dirname) %>
+      <% dirstack.pop %></ul></li>
+    <% end %>
+    <li>
+    <% if size.nil?  # This is a subdirectory. %>
+      <% dirstack.push(File.join(dirname, filename)) %>
+      <%= filename %>
+      <ul class="collection_files">
+    <% else %>
+      <%= link_to(filename,
+                  {controller: 'collections', action: 'show_file',
+                   uuid: @object.uuid, file: file_path,
+                   reader_token: params[:reader_token]},
+                  {title: "Download #{file_path}"}) %>
+      </li>
+    <% end %>
   <% end %>
-  </ul>
+  <%= raw(dirstack.map { |_| "</ul>" }.join("</li>")) %>
 <% else %>
   <p>No files in this collection.</p>
 <% end %>
+
+<div class="footer">
+<h2>About Arvados</h2>
+
+<p>Arvados is a free and open source software bioinformatics platform.
+To learn more, visit arvados.org.
+Arvados is not responsible for the files listed on this page.</p>
+</div>
+
 </body>
 </html>

commit 83277602806099428b859b88745ad85836cd6ee4
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Tue May 20 15:44:59 2014 -0400

    2764: Fixed active_all_collections scoped token to be able to access
    keep_disks.  Also some minor code cleanup in FileStreamer.

diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index 7af552d..3b4943f 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -207,20 +207,18 @@ class CollectionsController < ApplicationController
     end
     def each
       return unless @opts[:uuid] && @opts[:file]
-      env = Hash[ENV].
-        merge({
-                'ARVADOS_API_HOST' =>
-                arvados_api_client.arvados_v1_base.
-                sub(/\/arvados\/v1/, '').
-                sub(/^https?:\/\//, ''),
-                'ARVADOS_API_TOKEN' =>
-                @opts[:arvados_api_token],
-                'ARVADOS_API_HOST_INSECURE' =>
-                Rails.configuration.arvados_insecure_https ? 'true' : 'false'
-              })
+
+      env = Hash[ENV].dup
+
+      require 'uri'
+      u = URI.parse(arvados_api_client.arvados_v1_base)
+      env['ARVADOS_API_HOST'] = "#{u.host}:#{u.port}"
+      env['ARVADOS_API_TOKEN'] = @opts[:arvados_api_token]
+      env['ARVADOS_API_HOST_INSECURE'] = "true" if Rails.configuration.arvados_insecure_https
+
       IO.popen([env, 'arv-get', "#{@opts[:uuid]}/#{@opts[:file]}"],
                'rb') do |io|
-        while buf = io.read(2**20)
+        while buf = io.read(2**16)
           yield buf
         end
       end
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index aca6d7a..71b6388 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -63,7 +63,7 @@ active_all_collections:
   user: active
   api_token: activecollectionsabcdefghijklmnopqrstuvwxyz1234567
   expires_at: 2038-01-01 00:00:00
-  scopes: ["GET /arvados/v1/collections/"]
+  scopes: ["GET /arvados/v1/collections/", "GET /arvados/v1/keep_disks"]
 
 active_userlist:
   api_client: untrusted

commit a2c261dada5b47077c7930a2133bb000631fab66
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Tue May 20 15:00:42 2014 -0400

    2764: Fixed scopes_allow_request to use request.request_method (the effective
    HTTP method) instead of request.method (the actual HTTP method) because
    workbench uses POST even for GET.

diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 2488d83..82dd0ec 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -71,7 +71,7 @@ class ApiClientAuthorization < ArvadosModel
   end
 
   def scopes_allow_request?(request)
-    scopes_allow? [request.method, request.path].join(' ')
+    scopes_allow? [request.request_method, request.path].join(' ')
   end
 
   def logged_attributes

commit dffa580d39b977746f6950b835b78b949a862c9d
Author: Brett Smith <brett at curoverse.com>
Date:   Thu May 15 16:38:12 2014 -0400

    2764: Add wget-friendly Collections file page.
    
    This new route will become the way you share authless Collection links
    with others.  They can pass it to `wget -r` to download the whole
    collection, nicely organized, with nothing extraneous.  Since it
    doesn't try to load user information or look up related Arvados items,
    it can be rendered using an API token with a very narrow scope.
    
    Because wget respects robots.txt, this branch stops using that in
    favor of the corresponding <meta> tag.  The new view only limits
    indexing, so wget can follow the links on the page.
    
    Refs #2764.

diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index 7a8888e..7af552d 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -93,8 +93,7 @@ class CollectionsController < ApplicationController
   def show_file_links
     Thread.current[:reader_tokens] = [params[:reader_token]]
     find_object_by_uuid
-    show
-    render 'show'
+    render layout: false
   end
 
   def show_file
diff --git a/apps/workbench/app/views/collections/show_file_links.html.erb b/apps/workbench/app/views/collections/show_file_links.html.erb
new file mode 100644
index 0000000..9d61a6a
--- /dev/null
+++ b/apps/workbench/app/views/collections/show_file_links.html.erb
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>
+    <% if content_for? :page_title %>
+    <%= yield :page_title %> / <%= Rails.configuration.site_name %>
+    <% else %>
+    <%= Rails.configuration.site_name %>
+    <% end %>
+  </title>
+  <meta name="description" content="">
+  <meta name="author" content="">
+  <meta name="robots" content="NOINDEX">
+</head>
+<body>
+<% content_for :page_title do %>
+  <%= (@object.respond_to?(:properties) ? @object.properties[:page_title] : nil) ||
+        @object.friendly_link_name %>
+<% end %>
+
+<% if @object.andand.files.andand.any? %>
+  <% link_opts = {controller: 'collections', action: 'show_file',
+                  uuid: @object.uuid, reader_token: params[:reader_token]} %>
+  <ul>
+  <% @object.files.map { |spec|
+       CollectionsHelper::file_path(spec)
+     }.each do |path| %>
+    <li><%= link_to(path, link_opts.merge(file: path)) %></li>
+  <% end %>
+  </ul>
+<% else %>
+  <p>No files in this collection.</p>
+<% end %>
+</body>
+</html>
diff --git a/apps/workbench/app/views/layouts/application.html.erb b/apps/workbench/app/views/layouts/application.html.erb
index 660d2dc..a5460c2 100644
--- a/apps/workbench/app/views/layouts/application.html.erb
+++ b/apps/workbench/app/views/layouts/application.html.erb
@@ -14,6 +14,7 @@
   <link rel="shortcut icon" href="/favicon.ico" type="image/x-icon">
   <meta name="description" content="">
   <meta name="author" content="">
+  <meta name="robots" content="NOINDEX, NOFOLLOW">
   <%= stylesheet_link_tag    "application", :media => "all" %>
   <%= javascript_include_tag "application" %>
   <%= csrf_meta_tags %>
diff --git a/apps/workbench/public/robots.txt b/apps/workbench/public/robots.txt
index c6742d8..e69de29 100644
--- a/apps/workbench/public/robots.txt
+++ b/apps/workbench/public/robots.txt
@@ -1,2 +0,0 @@
-User-Agent: *
-Disallow: /
diff --git a/apps/workbench/test/integration/collections_test.rb b/apps/workbench/test/integration/collections_test.rb
index 4a33693..911daa0 100644
--- a/apps/workbench/test/integration/collections_test.rb
+++ b/apps/workbench/test/integration/collections_test.rb
@@ -3,7 +3,6 @@ require 'selenium-webdriver'
 require 'headless'
 
 class CollectionsTest < ActionDispatch::IntegrationTest
-
   def change_persist oldstate, newstate
     find "div[data-persistent-state='#{oldstate}']"
     page.assert_no_selector "div[data-persistent-state='#{newstate}']"
@@ -48,4 +47,30 @@ class CollectionsTest < ActionDispatch::IntegrationTest
     # isn't only showing up in an error message.
     assert(page.has_link?('foo'), "Collection page did not include file link")
   end
+
+  test "can download an entire collection with a reader token" do
+    uuid = api_fixture('collections')['foo_file']['uuid']
+    token = api_fixture('api_client_authorizations')['active_all_collections']['api_token']
+    url_head = "/collections/download/#{uuid}/#{token}/"
+    visit url_head
+    # It seems that Capybara can't inspect tags outside the body, so this is
+    # a very blunt approach.
+    assert_no_match(/<\s*meta[^>]+\bnofollow\b/i, page.html,
+                    "wget prohibited from recursing the collection page")
+    # TODO: When we can test against a Keep server, actually follow links
+    # and check their contents, rather than testing the href directly
+    # (this is too closely tied to implementation details).
+    hrefs = page.all('a').map do |anchor|
+      link = anchor[:href] || ''
+      if link.start_with? url_head
+        link[url_head.size .. -1]
+      elsif link.start_with? '/'
+        nil
+      else
+        link
+      end
+    end
+    assert_equal(['foo'], hrefs.compact.sort,
+                 "download page did provide strictly file links")
+  end
 end
diff --git a/services/api/test/fixtures/api_client_authorizations.yml b/services/api/test/fixtures/api_client_authorizations.yml
index 9901ec4..aca6d7a 100644
--- a/services/api/test/fixtures/api_client_authorizations.yml
+++ b/services/api/test/fixtures/api_client_authorizations.yml
@@ -58,6 +58,13 @@ admin_noscope:
   expires_at: 2038-01-01 00:00:00
   scopes: []
 
+active_all_collections:
+  api_client: untrusted
+  user: active
+  api_token: activecollectionsabcdefghijklmnopqrstuvwxyz1234567
+  expires_at: 2038-01-01 00:00:00
+  scopes: ["GET /arvados/v1/collections/"]
+
 active_userlist:
   api_client: untrusted
   user: active

commit 1924b28c8f715bc08752fb219d1fd8145ccfe84f
Author: Brett Smith <brett at curoverse.com>
Date:   Thu May 15 15:54:32 2014 -0400

    2764: Introduce show_file_links route.
    
    Right now this just takes advantage of the provided reader token, but
    we're going to extend it to provide a wget-friendly view.

diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index 43ff754..7a8888e 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -1,7 +1,8 @@
 class CollectionsController < ApplicationController
-  skip_around_filter :thread_with_mandatory_api_token, only: [:show_file]
-  skip_before_filter :find_object_by_uuid, only: [:provenance, :show_file]
-  skip_before_filter :check_user_agreements, only: [:show_file]
+  skip_around_filter(:thread_with_mandatory_api_token,
+                     only: [:show_file, :show_file_links])
+  skip_before_filter(:find_object_by_uuid,
+                     only: [:provenance, :show_file, :show_file_links])
 
   RELATION_LIMIT = 5
 
@@ -89,6 +90,13 @@ class CollectionsController < ApplicationController
     @request_url = request.url
   end
 
+  def show_file_links
+    Thread.current[:reader_tokens] = [params[:reader_token]]
+    find_object_by_uuid
+    show
+    render 'show'
+  end
+
   def show_file
     # We pipe from arv-get to send the file to the user.  Before we start it,
     # we ask the API server if the file actually exists.  This serves two
diff --git a/apps/workbench/config/routes.rb b/apps/workbench/config/routes.rb
index 30ac241..c12cc98 100644
--- a/apps/workbench/config/routes.rb
+++ b/apps/workbench/config/routes.rb
@@ -44,9 +44,10 @@ ArvadosWorkbench::Application.routes.draw do
   resources :collections do
     post 'set_persistent', on: :member
   end
-  get '/collections/:uuid/*file' => 'collections#show_file', :format => false
   get('/collections/download/:uuid/:reader_token/*file' => 'collections#show_file',
       format: false)
+  get '/collections/download/:uuid/:reader_token' => 'collections#show_file_links'
+  get '/collections/:uuid/*file' => 'collections#show_file', :format => false
   resources :folders do
     match 'remove/:item_uuid', on: :member, via: :delete, action: :remove_item
   end
diff --git a/apps/workbench/test/functional/collections_controller_test.rb b/apps/workbench/test/functional/collections_controller_test.rb
index b31e718..19d08d7 100644
--- a/apps/workbench/test/functional/collections_controller_test.rb
+++ b/apps/workbench/test/functional/collections_controller_test.rb
@@ -89,11 +89,10 @@ class CollectionsControllerTest < ActionController::TestCase
   end
 
   test "viewing collection files with a reader token" do
-    skip  # Need a new route+view for this.
     params = collection_params(:foo_file)
     params[:reader_token] =
       api_fixture('api_client_authorizations')['active']['api_token']
-    get(:show, params)
+    get(:show_file_links, params)
     assert_response :success
     assert_equal([['.', 'foo', 3]], assigns(:object).files)
     assert_no_session

commit 0bec8ec8a130cb3af013b6097cf322938fce5671
Author: Brett Smith <brett at curoverse.com>
Date:   Wed May 21 15:16:34 2014 -0400

    2765: Tear out general Workbench reader_tokens support.
    
    Refs #2765.  This approach is not tenable in its current form, and the
    callbacks are making further development confusing, so I'm getting rid
    of them.
    
    Conflicts:
    	apps/workbench/test/functional/collections_controller_test.rb

diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index b62838f..ade586c 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -7,12 +7,10 @@ class ApplicationController < ActionController::Base
   ERROR_ACTIONS = [:render_error, :render_not_found]
 
   around_filter :thread_clear
-  around_filter(:thread_with_mandatory_api_token,
-                except: [:index, :show] + ERROR_ACTIONS)
+  around_filter :thread_with_mandatory_api_token, except: ERROR_ACTIONS
   around_filter :thread_with_optional_api_token
   before_filter :check_user_agreements, except: ERROR_ACTIONS
   before_filter :check_user_notifications, except: ERROR_ACTIONS
-  around_filter :using_reader_tokens, only: [:index, :show]
   before_filter :find_object_by_uuid, except: [:index] + ERROR_ACTIONS
   before_filter :check_my_folders, :except => ERROR_ACTIONS
   theme :select_theme
@@ -214,23 +212,6 @@ class ApplicationController < ActionController::Base
     false  # For convenience to return from callbacks
   end
 
-  def using_reader_tokens(login_optional=false)
-    if params[:reader_tokens].is_a?(Array) and params[:reader_tokens].any?
-      Thread.current[:reader_tokens] = params[:reader_tokens]
-    end
-    begin
-      yield
-    rescue ArvadosApiClient::NotLoggedInException
-      if login_optional
-        raise
-      else
-        return redirect_to_login
-      end
-    ensure
-      Thread.current[:reader_tokens] = nil
-    end
-  end
-
   def using_specific_api_token(api_token)
     start_values = {}
     [:arvados_api_token, :user].each do |key|
diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index 370c681..43ff754 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -95,7 +95,8 @@ class CollectionsController < ApplicationController
     # purposes: it lets us return a useful status code for common errors, and
     # helps us figure out which token to provide to arv-get.
     coll = nil
-    usable_token = find_usable_token do
+    tokens = [Thread.current[:arvados_api_token], params[:reader_token]].compact
+    usable_token = find_usable_token(tokens) do
       coll = Collection.find(params[:uuid])
     end
     if usable_token.nil?
@@ -148,18 +149,14 @@ class CollectionsController < ApplicationController
 
   protected
 
-  def find_usable_token
-    # Iterate over every token available to make it the current token and
+  def find_usable_token(token_list)
+    # Iterate over every given token to make it the current token and
     # yield the given block.
     # If the block succeeds, return the token it used.
     # Otherwise, render an error response based on the most specific
     # error we encounter, and return nil.
-    read_tokens = [Thread.current[:arvados_api_token]].compact
-    if params[:reader_tokens].is_a? Array
-      read_tokens += params[:reader_tokens]
-    end
     most_specific_error = [401]
-    read_tokens.each do |api_token|
+    token_list.each do |api_token|
       using_specific_api_token(api_token) do
         begin
           yield
diff --git a/apps/workbench/config/routes.rb b/apps/workbench/config/routes.rb
index 2255856..30ac241 100644
--- a/apps/workbench/config/routes.rb
+++ b/apps/workbench/config/routes.rb
@@ -45,6 +45,8 @@ ArvadosWorkbench::Application.routes.draw do
     post 'set_persistent', on: :member
   end
   get '/collections/:uuid/*file' => 'collections#show_file', :format => false
+  get('/collections/download/:uuid/:reader_token/*file' => 'collections#show_file',
+      format: false)
   resources :folders do
     match 'remove/:item_uuid', on: :member, via: :delete, action: :remove_item
   end
diff --git a/apps/workbench/test/functional/collections_controller_test.rb b/apps/workbench/test/functional/collections_controller_test.rb
index 4f93450..b31e718 100644
--- a/apps/workbench/test/functional/collections_controller_test.rb
+++ b/apps/workbench/test/functional/collections_controller_test.rb
@@ -88,29 +88,15 @@ class CollectionsControllerTest < ActionController::TestCase
                     "controller did not find related log")
   end
 
-  test "viewing a collection with a reader token" do
+  test "viewing collection files with a reader token" do
+    skip  # Need a new route+view for this.
     params = collection_params(:foo_file)
-    params[:reader_tokens] =
-      [api_fixture('api_client_authorizations')['active']['api_token']]
-    show_collection(params)
-    assert_equal([['.', 'foo', 3]], assigns(:object).files)
-    assert_no_session
-  end
-
-  test "viewing the index with a reader token" do
-    params = {reader_tokens:
-      [api_fixture('api_client_authorizations')['spectator']['api_token']]
-    }
-    get(:index, params)
+    params[:reader_token] =
+      api_fixture('api_client_authorizations')['active']['api_token']
+    get(:show, params)
     assert_response :success
+    assert_equal([['.', 'foo', 3]], assigns(:object).files)
     assert_no_session
-    listed_collections = assigns(:collections).map { |c| c.uuid }
-    assert_includes(listed_collections,
-                    api_fixture('collections')['bar_file']['uuid'],
-                    "spectator reader token didn't list bar file")
-    refute_includes(listed_collections,
-                    api_fixture('collections')['foo_file']['uuid'],
-                    "spectator reader token listed foo file")
   end
 
   test "getting a file from Keep" do
@@ -139,7 +125,7 @@ class CollectionsControllerTest < ActionController::TestCase
   test "getting a file from Keep with a good reader token" do
     params = collection_params(:foo_file, 'foo')
     read_token = api_fixture('api_client_authorizations')['active']['api_token']
-    params[:reader_tokens] = [read_token]
+    params[:reader_token] = read_token
     get(:show_file, params)
     assert_response :success
     assert_equal(expected_contents(params, read_token), @response.body,
@@ -150,9 +136,8 @@ class CollectionsControllerTest < ActionController::TestCase
 
   test "trying to get from Keep with an unscoped reader token prompts login" do
     params = collection_params(:foo_file, 'foo')
-    read_token =
+    params[:reader_token] =
       api_fixture('api_client_authorizations')['active_noscope']['api_token']
-    params[:reader_tokens] = [read_token]
     get(:show_file, params)
     assert_response :redirect
   end
@@ -161,7 +146,7 @@ class CollectionsControllerTest < ActionController::TestCase
     params = collection_params(:foo_file, 'foo')
     sess = session_for(:expired)
     read_token = api_fixture('api_client_authorizations')['active']['api_token']
-    params[:reader_tokens] = [read_token]
+    params[:reader_token] = read_token
     get(:show_file, params, sess)
     assert_response :success
     assert_equal(expected_contents(params, read_token), @response.body,
diff --git a/apps/workbench/test/test_helper.rb b/apps/workbench/test/test_helper.rb
index c1eed5c..e833f97 100644
--- a/apps/workbench/test/test_helper.rb
+++ b/apps/workbench/test/test_helper.rb
@@ -36,6 +36,7 @@ class ActiveSupport::TestCase
 
   def teardown
     Thread.current[:arvados_api_token] = nil
+    Thread.current[:reader_tokens] = nil
     super
   end
 end

commit fa6b0b65fb71b66e36f982f4f25e9751672d0834
Author: Brett Smith <brett at curoverse.com>
Date:   Tue May 20 15:15:11 2014 -0400

    2764: Reset browser session between Workbench integration tests.
    
    Peter and I just debugged an issue where a test was falsely passing
    because the session had authorization state in it.  This prevents it
    from passing.

diff --git a/apps/workbench/test/integration_helper.rb b/apps/workbench/test/integration_helper.rb
index a8788ce..93455ee 100644
--- a/apps/workbench/test/integration_helper.rb
+++ b/apps/workbench/test/integration_helper.rb
@@ -25,6 +25,11 @@ class ActionDispatch::IntegrationTest
 
   @@API_AUTHS = self.api_fixture('api_client_authorizations')
 
+  def setup
+    reset_session!
+    super
+  end
+
   def page_with_token(token, path='/')
     # Generate a page path with an embedded API token.
     # Typical usage: visit page_with_token('token_name', page)

commit 661b2bcd6269fae643fd7d0c46649715716959eb
Author: Brett Smith <brett at curoverse.com>
Date:   Wed May 21 13:30:05 2014 -0400

    2753: Properly close entire Collection file tree in Workbench.

diff --git a/apps/workbench/app/views/collections/_show_files.html.erb b/apps/workbench/app/views/collections/_show_files.html.erb
index 38c5d00..c5c1279 100644
--- a/apps/workbench/app/views/collections/_show_files.html.erb
+++ b/apps/workbench/app/views/collections/_show_files.html.erb
@@ -64,5 +64,5 @@
       </li>
     <% end  # if file or directory %>
   <% end  # file_tree.each %>
-  </ul>
+  <%= raw(dirstack.map { |_| "</ul>" }.join("</li>")) %>
 <% end  # if file_tree %>

commit f876a32276a73d89ab695e5954c9ea3816b38db9
Author: Brett Smith <brett at curoverse.com>
Date:   Tue May 6 11:30:58 2014 -0400

    1904: Make Keep file EPERM test stricter.
    
    Refs #1904.  Tom said that we want the security benefits of a 404
    result, so make sure that's exactly what we get.

diff --git a/apps/workbench/test/functional/collections_controller_test.rb b/apps/workbench/test/functional/collections_controller_test.rb
index 7a15161..4f93450 100644
--- a/apps/workbench/test/functional/collections_controller_test.rb
+++ b/apps/workbench/test/functional/collections_controller_test.rb
@@ -126,7 +126,7 @@ class CollectionsControllerTest < ActionController::TestCase
     params = collection_params(:foo_file, 'foo')
     sess = session_for(:spectator)
     get(:show_file, params, sess)
-    assert_includes([403, 404], @response.code.to_i)
+    assert_response 404
   end
 
   test "trying to get a nonexistent file from Keep returns a 404" do

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list