documentcloud · reefdog · Dec 22, 2015 · Dec 22, 2015 · Dec 22, 2015 · Dec 29, 2015
diff --git a/Gemfile b/Gemfile
@@ -39,6 +39,7 @@ gem 'jammit',                 github: 'documentcloud/jammit', branch: 'master'
 gem 'nokogiri',               '~> 1.6.0'
 gem 'net-ssh-shell'
 gem 'country_select',         '~> 2.2.0'
+gem 'addressable',            '~> 2.4.0'
 gem 'bootstrap_form',         github: 'documentcloud/rails-bootstrap-forms',
                               branch: 'bootstrap-v4'
 

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -378,6 +378,7 @@ PLATFORMS
 DEPENDENCIES
   actionpack-page_caching
   active_model_serializers (~> 0.9.0)
+  addressable (~> 2.4.0)
   aws-sdk (~> 1.30)
   bcrypt (~> 3.1.1)
   bootstrap_form!

diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
@@ -171,7 +171,7 @@ def destroy_project
 
   def oembed
     # get the target url and turn it into a manipulable object.
-    url = URI.parse(CGI.unescape(params[:url])) rescue nil
+    url = Addressable::URI.parse(CGI.unescape(params[:url])) rescue nil
     return bad_request if params[:url].blank? or !url
 
     # Use the rails router to identify whether a URL is an embeddable resource
@@ -241,12 +241,12 @@ def resource_embeddable?(resource_params)
     (
       (
         %w[documents pages].include?(resource_params[:controller]) and
-        resource_params[:id] =~ DC::Validators::SLUG # and
-        # Document.accessible(nil, nil).exists?(resource_params[:id])
+        resource_params[:id].force_encoding('utf-8') =~ DC::Validators::SLUG # and
+        # Document.accessible(nil, nil).exists?(resource_params[:id].to_i) 
       ) or
       (
         resource_params[:controller] == "annotations" and
-        resource_params[:document_id] =~ DC::Validators::SLUG # and
+        resource_params[:document_id].force_encoding('utf-8') =~ DC::Validators::SLUG
         # Annotation.accessible(nil).exists?(resource_params[:id])
       )
     )

diff --git a/app/models/document.rb b/app/models/document.rb
@@ -513,8 +513,8 @@ def annotations_path
     File.join(path, 'annotations')
   end
 
-  def canonical_id
-    "#{id}-#{slug}"
+  def canonical_id(options={})
+    "#{id}-#{options[:escaped] ? CGI.escape(slug) : slug}"
   end
 
   def canonical_path(format = :json)

diff --git a/test/controllers/oembed_controller_test.rb b/test/controllers/oembed_controller_test.rb
@@ -7,6 +7,12 @@ class OembedControllerTest < ActionController::TestCase
   include Rails.application.routes.url_helpers
   default_url_options[:host] = DC::CONFIG['server_root']
 
+  # TODO: Clean up all these [JR]
+  let(:valid_resource_url) { CGI.escape(url_for :controller => 'documents', :action => 'show', :id => '1-is-the-lonelist-number', :format => 'html') }
+  let(:unicode_resource_url) { CGI.escape(url_for :controller => 'documents', :action => 'show', :id => '2-不', :format => 'html') }
+  let(:missing_resource_url) { CGI.escape(url_for :controller => 'documents', :action => 'show', :id => '3-the-magic-number', :format => 'html') }
+  let(:unsupported_resource_url) { CGI.escape(url_for :controller => 'home', :action => 'index') }
+  let(:external_resource_url) { CGI.escape('http://www2.warnerbros.com/spacejam/movie/jam.htm') }
   let(:public_document_url) { CGI.escape(documents(:tv_manual).canonical_url(:html)) }
   let(:private_document_url) { CGI.escape(documents(:top_secret).canonical_url(:html)) }
   let(:missing_url) { CGI.escape(url_for controller: 'documents', action: 'show', id: '404-not-found', format: 'html') }
@@ -21,6 +27,11 @@ class OembedControllerTest < ActionController::TestCase
     assert_equal DC.server_root, json_body['provider_url']
   end
 
+  it "should correctly parse Unicode slugs" do
+    get :oembed, :format => "json", :url => unicode_resource_url
+    assert_response :success
+  end
+
   it "should require a URL parameter" do
     get :oembed, format: "json"
     assert_response 400

diff --git a/test/controllers/search_controller_test.rb b/test/controllers/search_controller_test.rb
@@ -7,7 +7,7 @@ def test_documents
     login_account!
     get :documents, :format=>:json, :per_page=>10, :order=>'score', :mentions=>3, :q=>''
     assert_response :success
-    assert_equal [ secret_doc.id, doc.id ], json_body['documents'].map{|d|d['id']}.sort
+    assert_equal [ secret_doc.id, unicode_doc.id, doc.id ], json_body['documents'].map{|d|d['id']}.sort
   end
 
   def test_solr_searching

diff --git a/test/fixtures/documents.yml b/test/fixtures/documents.yml
@@ -25,6 +25,32 @@ tv_manual:
   created_at: <%= Time.now - 1.month - 2.days%>
   updated_at: <%= Time.now - 1.month - 1.days%>
 
+unicode:
+  publication_date:
+  text_changed: f
+  title: Меня зовут Кид Рок
+  original_extension: pdf
+  description: ""
+  hit_count: 1001
+  slug: Меня-зовут-Кид-Рок
+  reviewer_count: "0"
+  source: ""
+  page_count: 68
+  organization: tribune
+  calais_id:
+  public_note_count: "102"
+  publish_at:
+  access: <%= DC::Access::PUBLIC %>
+  char_count: "146055"
+  related_article:
+  language: ru
+  file_hash: a483ba7b72e0571f23d61f5233228c9540ce325b
+  file_size: "2662279"
+  remote_url: http://example.com/my_precious
+  detected_remote_url:
+  account: louis
+  created_at: <%= Time.now - 1.month - 2.days%>
+  updated_at: <%= Time.now - 1.month - 1.days%>
 
 top_secret:
   publication_date:

diff --git a/test/lib/statistics_test.rb b/test/lib/statistics_test.rb
@@ -7,7 +7,7 @@ class StatisticsTest < ActiveSupport::TestCase
   MONTH_DATE  = Date.today - 1.month - 2.day
   def test_daily_documents
     docs = DC::Statistics.daily_documents( MONTH_DATE )
-    assert_equal 1, docs.detect{ |day, count| day.to_date == MONTH_DATE }.last
+    assert_equal 2, docs.detect{ |day, count| day.to_date == MONTH_DATE }.last
   end
 
   def test_daily_pages
@@ -17,12 +17,12 @@ def test_daily_pages
 
   def test_weekly_documents
     docs = DC::Statistics.weekly_documents( Date.parse("2013-01-01") )
-    assert_equal [1, 1], docs.values
+    assert_equal [1, 2], docs.values
   end
 
   def test_weekly_pages
     pgs = DC::Statistics.weekly_pages
-    assert_equal [68, 68], pgs.values
+    assert_equal [68, 136], pgs.values
   end
 
   def test_daily_hits_on_documents
@@ -61,7 +61,7 @@ def test_pages_since
 
   def test_public_documents_per_account
     counts =  DC::Statistics.public_documents_per_account
-    assert_equal 1, counts[ louis.id ]
+    assert_equal 2, counts[ louis.id ]
   end
 
   def test_private_documents_per_account
@@ -71,11 +71,11 @@ def test_private_documents_per_account
 
   def test_pages_per_account
     counts =  DC::Statistics.pages_per_account
-    assert_equal 136, counts[ louis.id ]
+    assert_equal 204, counts[ louis.id ]
   end
 
   def test_total_pages
-    assert_equal 136, DC::Statistics.total_pages
+    assert_equal 204, DC::Statistics.total_pages
   end
 
   def test_by_the_numbers
@@ -85,8 +85,8 @@ def test_by_the_numbers
     assert_equal( {:total=>4,   :day=>4, :week=>4, :month=>4,  :half_year=>4}   , nums["All Accounts"] )
     assert_equal( {:total=>1,   :day=>1, :week=>1, :month=>1,  :half_year=>1}   , nums["Active Accounts"] )
     assert_equal( {:total=>0,   :day=>0, :week=>0, :month=>0,  :half_year=>0}   , nums["Reviewers"] )
-    assert_equal( {:total=>2,   :day=>0, :week=>0, :month=>1,  :half_year=>2}   , nums["Documents"] )
-    assert_equal( {:total=>136, :day=>0, :week=>0, :month=>68, :half_year=>136} , nums["Pages"] )
+    assert_equal( {:total=>3,   :day=>0, :week=>0, :month=>1,  :half_year=>3}   , nums["Documents"] )
+    assert_equal( {:total=>204, :day=>0, :week=>0, :month=>68, :half_year=>204} , nums["Pages"] )
     assert_equal( {:total=>3,   :day=>3, :week=>3, :month=>3,  :half_year=>3}   , nums["Notes"] )
   end
 
@@ -97,7 +97,7 @@ def test_pages_per_minute
   def test_documents_by_access
     docs = DC::Statistics.documents_by_access
     assert_equal 1, docs[ Document::PRIVATE ]
-    assert_equal 1, docs[ Document::PUBLIC  ]
+    assert_equal 2, docs[ Document::PUBLIC  ]
   end
 
   def test_averate_page_count
@@ -109,7 +109,7 @@ def test_average_entity_count
   end
 
   def test_embedded_document_count
-    assert_equal 1, DC::Statistics.embedded_document_count
+    assert_equal 2, DC::Statistics.embedded_document_count
   end
 
   def test_remote_url_hits_last_week

diff --git a/test/models/organization_test.rb b/test/models/organization_test.rb
@@ -35,7 +35,7 @@ class OrganizationTest < ActiveSupport::TestCase
   end
 
   it "counts documents" do
-    assert_equal 2, tribune.document_count
+    assert_equal 3, tribune.document_count
   end
 
   it "returns an accounts role" do

diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -76,6 +76,7 @@ class ActiveSupport::TestCase
   fixtures :all
 
   let (:doc) { documents(:tv_manual) }
+  let (:unicode_doc){ documents(:unicode)}
   let (:secret_doc){ documents(:top_secret)}
   let (:louis){ accounts(:louis) }
   let (:tribune){ organizations(:tribune) }