From a8261ab9f189f76d9b8f6467db4abb97c4461492 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Tue, 13 Feb 2018 16:56:43 +1300 Subject: [PATCH 01/15] save xml document as class attribute once xml has been parsed --- lib/supplejack_common/xml_helpers/xml_document_methods.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/supplejack_common/xml_helpers/xml_document_methods.rb b/lib/supplejack_common/xml_helpers/xml_document_methods.rb index 4f78cba..d1a7e3f 100644 --- a/lib/supplejack_common/xml_helpers/xml_document_methods.rb +++ b/lib/supplejack_common/xml_helpers/xml_document_methods.rb @@ -21,8 +21,9 @@ def xml_records(url) xml_nodes = [] with_each_file(url) do |file| document = parse_document(file) + self._document = document xml_nodes += document.xpath(self._record_selector, self._namespaces).map {|node| new(node, url) } - if pagination_options + if pagination_options.include?(:total_selector) if self.pagination_options[:total_selector].start_with?("/") self._total_results ||= document.xpath(self.pagination_options[:total_selector]).text.to_i else From 28289281366733145a1357871d8753b8203413c9 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Tue, 13 Feb 2018 16:57:11 +1300 Subject: [PATCH 02/15] add next page token method to xml base for tokenised pagination --- lib/supplejack_common/xml/base.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/supplejack_common/xml/base.rb b/lib/supplejack_common/xml/base.rb index f435dd5..a3d9df0 100644 --- a/lib/supplejack_common/xml/base.rb +++ b/lib/supplejack_common/xml/base.rb @@ -18,8 +18,17 @@ class Base < SupplejackCommon::Base class_attribute :_record_selector class_attribute :_record_format class_attribute :_total_results + class_attribute :_document class << self + def record_selector(xpath) + self._record_selector = xpath + end + + def next_page_token(next_page_token_location) + _document.xpath('//o:resumptionToken', self._namespaces).first.text + end + def records(options={}) options.reverse_merge!(limit: nil) klass = !!self._sitemap_entry_selector ? SupplejackCommon::Sitemap::PaginatedCollection : SupplejackCommon::PaginatedCollection @@ -34,15 +43,13 @@ def record_format(format) self._record_format = format.to_sym end - def record_selector(xpath) - self._record_selector = xpath - end def clear_definitions super self._record_selector = nil self._total_results = nil self._record_format = nil + self._document = nil end def total_results(_total_selector) From 238517c243c435f3aa7a568487a757ccc50b7674 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Tue, 13 Feb 2018 16:57:45 +1300 Subject: [PATCH 03/15] changes to pagination collection so xml can used tokenised pagination --- lib/supplejack_common/paginated_collection.rb | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/supplejack_common/paginated_collection.rb b/lib/supplejack_common/paginated_collection.rb index a1abf2d..9aeb054 100644 --- a/lib/supplejack_common/paginated_collection.rb +++ b/lib/supplejack_common/paginated_collection.rb @@ -28,6 +28,7 @@ def initialize(klass, pagination_options={}, options={}) @tokenised = pagination_options[:tokenised] || false @next_page_token_location = pagination_options[:next_page_token_location] @total_selector = pagination_options[:total_selector] + @initial_param = pagination_options[:initial_param] @options = options @counter = 0 @@ -56,11 +57,18 @@ def each(&block) private + def initial_url(url, joiner) + url = "#{url}#{joiner}#{@initial_param}" + @initial_param = nil + url + end + def next_url(url) if paginated? joiner = url.match(/\?/) ? "&" : "?" if @tokenised @page = self.klass._document.present? ? self.klass.next_page_token(@next_page_token_location) : nil + return initial_url(url, joiner) if @initial_param.present? url = "#{url}#{joiner}#{url_options.to_query}" else url = "#{url}#{joiner}#{url_options.to_query}" @@ -73,7 +81,10 @@ def next_url(url) end def url_options - {page_parameter => page, per_page_parameter => per_page} + options = {} + options[page_parameter] = page if page_parameter.present? + options[per_page_parameter] = per_page if per_page_parameter.present? + options end def page_pagination? From c7b69c3b202896d2e7b456354230630d805bca74 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Wed, 14 Feb 2018 08:55:19 +1300 Subject: [PATCH 04/15] add class attribute to sitemap base so xml document methods can be used everywhere _document necessary for paginated collections. --- lib/supplejack_common/sitemap/base.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/supplejack_common/sitemap/base.rb b/lib/supplejack_common/sitemap/base.rb index 71464d2..05b9c9f 100644 --- a/lib/supplejack_common/sitemap/base.rb +++ b/lib/supplejack_common/sitemap/base.rb @@ -14,6 +14,7 @@ class Base < SupplejackCommon::Base class_attribute :_record_selector class_attribute :_namespaces + class_attribute :_document class << self def fetch_entries(url=nil) From bb2e18a094559d24f4163368e88b913f00899025 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Wed, 14 Feb 2018 08:55:55 +1300 Subject: [PATCH 05/15] lonely operator in xml document methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit same as if pagination_options && pagination_options.include?… --- lib/supplejack_common/xml_helpers/xml_document_methods.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/supplejack_common/xml_helpers/xml_document_methods.rb b/lib/supplejack_common/xml_helpers/xml_document_methods.rb index d1a7e3f..b68da97 100644 --- a/lib/supplejack_common/xml_helpers/xml_document_methods.rb +++ b/lib/supplejack_common/xml_helpers/xml_document_methods.rb @@ -23,7 +23,7 @@ def xml_records(url) document = parse_document(file) self._document = document xml_nodes += document.xpath(self._record_selector, self._namespaces).map {|node| new(node, url) } - if pagination_options.include?(:total_selector) + if pagination_options&.include?(:total_selector) if self.pagination_options[:total_selector].start_with?("/") self._total_results ||= document.xpath(self.pagination_options[:total_selector]).text.to_i else From 8a235eb73be3007e5c3c5d999863cbc99fb631d3 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Wed, 14 Feb 2018 09:46:10 +1300 Subject: [PATCH 06/15] specs for initial params in paginated collection --- .../paginated_collection_spec.rb | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/spec/supplejack_common/paginated_collection_spec.rb b/spec/supplejack_common/paginated_collection_spec.rb index 246b9d2..6e663c3 100644 --- a/spec/supplejack_common/paginated_collection_spec.rb +++ b/spec/supplejack_common/paginated_collection_spec.rb @@ -100,7 +100,7 @@ context "tokenised pagination" do let(:params) { { - page_parameter: "page", + page_parameter: "page-parameter", type: "item", tokenised: true, per_page_parameter: "per_page", @@ -115,10 +115,34 @@ SupplejackCommon::Base.stub(:_document) {true} end - it "does some things" do - expect(collection.send(:next_url, "http://go.gle/?sort=asc")).to eq "http://go.gle/?sort=asc&page=abc_1234&per_page=5" + it "generates the next url" do + expect(collection.send(:next_url, "http://go.gle/?sort=asc")).to eq "http://go.gle/?sort=asc&page-parameter=abc_1234&per_page=5" end end + + context "with initial parameter" do + let(:params) { { + page_parameter: "page-parameter", + type: "item", + tokenised: true, + initial_param: 'initial-paramater=true' + }} + let(:collection) { klass.new(SupplejackCommon::Base, params, {limit: 1}) } + + before do + SupplejackCommon::Base.stub(:next_page_token) {'abc_1234'} + SupplejackCommon::Base.stub(:_document) {true} + end + + it "generates a url with an initial parameter" do + expect(collection.send(:next_url, "http://go.gle/?sort=asc")).to eq "http://go.gle/?sort=asc&initial-paramater=true" + end + + it 'generates next url without initial parameter after the first call' do + expect(collection.send(:next_url, "http://go.gle/?sort=asc")).to eq "http://go.gle/?sort=asc&initial-paramater=true" + expect(collection.send(:next_url, "http://go.gle/?sort=asc")).to eq "http://go.gle/?sort=asc&page-parameter=abc_1234" + end + end end describe "#url_options" do From 15d7d7bad7e5029e4984e4d56925cd2c873d5be8 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Wed, 14 Feb 2018 09:49:22 +1300 Subject: [PATCH 07/15] spec for paginated collections url_options to removing nil keys in hash --- spec/supplejack_common/paginated_collection_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/supplejack_common/paginated_collection_spec.rb b/spec/supplejack_common/paginated_collection_spec.rb index 6e663c3..5f472c7 100644 --- a/spec/supplejack_common/paginated_collection_spec.rb +++ b/spec/supplejack_common/paginated_collection_spec.rb @@ -149,6 +149,11 @@ it "returns a hash with the url options" do collection.send(:url_options).should eq({"page" => 1, "per_page" => 5}) end + + it "removes nil keys from the hash of url options" do + collection = klass.new(SupplejackCommon::Base, {page_parameter: "page", page: 1, type: "item", per_page_parameter: nil}) + collection.send(:url_options).should eq({"page" => 1}) + end end describe "#current_page" do From 1d5bef241f507ca910d835e1fea7e06f5a974020 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Wed, 14 Feb 2018 10:29:31 +1300 Subject: [PATCH 08/15] next page token method updated to use parameter --- lib/supplejack_common/xml/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/supplejack_common/xml/base.rb b/lib/supplejack_common/xml/base.rb index a3d9df0..063d846 100644 --- a/lib/supplejack_common/xml/base.rb +++ b/lib/supplejack_common/xml/base.rb @@ -26,7 +26,7 @@ def record_selector(xpath) end def next_page_token(next_page_token_location) - _document.xpath('//o:resumptionToken', self._namespaces).first.text + _document.xpath(next_page_token_location, self._namespaces).first.text end def records(options={}) From 41db8cac333ca6fc7949926c0a5fae6435d18c6e Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Wed, 14 Feb 2018 10:29:57 +1300 Subject: [PATCH 09/15] specs for record selector methods and next page token methods on xml base class --- spec/supplejack_common/xml/base_spec.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/spec/supplejack_common/xml/base_spec.rb b/spec/supplejack_common/xml/base_spec.rb index fe72cd2..8597bbb 100644 --- a/spec/supplejack_common/xml/base_spec.rb +++ b/spec/supplejack_common/xml/base_spec.rb @@ -16,6 +16,20 @@ klass.clear_definitions end + describe ".record_selector" do + it 'assignes the record selector xpath class attributed' do + klass.record_selector("//o:ListRecords/o:record") + expect(klass._record_selector).to eq "//o:ListRecords/o:record" + end + end + + describe "next_page_token" do + it 'returns the next page token from the document of xml' do + klass._document = Nokogiri::XML.parse "token" + expect(klass.next_page_token('//NextPageToken')).to eq 'token' + end + end + describe ".records" do it "returns an object of type SupplejackCommon::Sitemap::PaginatedCollection when sitemap_entry_selector is set" do klass.should_receive(:_sitemap_entry_selector).twice.and_return("//loc") From 08a56e31a40b8bcd54fab164a24811d8ca8ea2c6 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Wed, 14 Feb 2018 10:39:13 +1300 Subject: [PATCH 10/15] remove tokenised attribute and use type = 'tokenised' in parser --- lib/supplejack_common/paginated_collection.rb | 9 ++++----- spec/supplejack_common/paginated_collection_spec.rb | 6 ++---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/supplejack_common/paginated_collection.rb b/lib/supplejack_common/paginated_collection.rb index 9aeb054..0e12507 100644 --- a/lib/supplejack_common/paginated_collection.rb +++ b/lib/supplejack_common/paginated_collection.rb @@ -25,7 +25,6 @@ def initialize(klass, pagination_options={}, options={}) @per_page = pagination_options[:per_page] @page = pagination_options[:page] @type = pagination_options[:type] - @tokenised = pagination_options[:tokenised] || false @next_page_token_location = pagination_options[:next_page_token_location] @total_selector = pagination_options[:total_selector] @initial_param = pagination_options[:initial_param] @@ -66,7 +65,7 @@ def initial_url(url, joiner) def next_url(url) if paginated? joiner = url.match(/\?/) ? "&" : "?" - if @tokenised + if tokenised? @page = self.klass._document.present? ? self.klass.next_page_token(@next_page_token_location) : nil return initial_url(url, joiner) if @initial_param.present? url = "#{url}#{joiner}#{url_options.to_query}" @@ -112,18 +111,18 @@ def increment_page_counter! end def more_results? - if @tokenised + if tokenised? return self.klass.next_page_token(@next_page_token_location).present? end current_page <= total_pages end def paginated? - (page && per_page) || @tokenised + (page && per_page) || tokenised? end def tokenised? - @tokenised + @type == 'tokenised' end def yield_from_records(&block) diff --git a/spec/supplejack_common/paginated_collection_spec.rb b/spec/supplejack_common/paginated_collection_spec.rb index 5f472c7..5fe2aa2 100644 --- a/spec/supplejack_common/paginated_collection_spec.rb +++ b/spec/supplejack_common/paginated_collection_spec.rb @@ -101,8 +101,7 @@ context "tokenised pagination" do let(:params) { { page_parameter: "page-parameter", - type: "item", - tokenised: true, + type: "tokenised", per_page_parameter: "per_page", per_page: 5, next_page_token_location: "next_page_token", @@ -123,8 +122,7 @@ context "with initial parameter" do let(:params) { { page_parameter: "page-parameter", - type: "item", - tokenised: true, + type: "tokenised", initial_param: 'initial-paramater=true' }} let(:collection) { klass.new(SupplejackCommon::Base, params, {limit: 1}) } From 4c2d383af536ecd24e4e251d085e56f6193eb4fe Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Wed, 14 Feb 2018 13:30:34 +1300 Subject: [PATCH 11/15] code review tweak to xml base.rb --- lib/supplejack_common/xml/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/supplejack_common/xml/base.rb b/lib/supplejack_common/xml/base.rb index 063d846..b7d15a2 100644 --- a/lib/supplejack_common/xml/base.rb +++ b/lib/supplejack_common/xml/base.rb @@ -26,7 +26,7 @@ def record_selector(xpath) end def next_page_token(next_page_token_location) - _document.xpath(next_page_token_location, self._namespaces).first.text + self._document.xpath(next_page_token_location, self._namespaces).first.text end def records(options={}) From 4bb056a6e4de0b45819e9c81c3a4d2f53ab03376 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Wed, 14 Feb 2018 13:38:45 +1300 Subject: [PATCH 12/15] code review --- lib/supplejack_common/sitemap/base.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/supplejack_common/sitemap/base.rb b/lib/supplejack_common/sitemap/base.rb index 05b9c9f..67bdc56 100644 --- a/lib/supplejack_common/sitemap/base.rb +++ b/lib/supplejack_common/sitemap/base.rb @@ -1,9 +1,9 @@ # The Supplejack Common code is Crown copyright (C) 2014, New Zealand Government, -# and is licensed under the GNU General Public License, version 3. -# See https://github.com/DigitalNZ/supplejack for details. -# -# Supplejack was created by DigitalNZ at the National Library of NZ and the Department of Internal Affairs. -# http://digitalnz.org/supplejack +# and is licensed under the GNU General Public License, version 3. +# See https://github.com/DigitalNZ/supplejack for details. +# +# Supplejack was created by DigitalNZ at the National Library of NZ and the Department of Internal Affairs. +# http://digitalnz.org/supplejack module SupplejackCommon module Sitemap From 4235f4706689e7ae1fbe6dae6a32935a56b5fba8 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Wed, 14 Feb 2018 13:45:23 +1300 Subject: [PATCH 13/15] better format for next_url method --- lib/supplejack_common/paginated_collection.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/supplejack_common/paginated_collection.rb b/lib/supplejack_common/paginated_collection.rb index 0e12507..d8f4a0f 100644 --- a/lib/supplejack_common/paginated_collection.rb +++ b/lib/supplejack_common/paginated_collection.rb @@ -67,12 +67,13 @@ def next_url(url) joiner = url.match(/\?/) ? "&" : "?" if tokenised? @page = self.klass._document.present? ? self.klass.next_page_token(@next_page_token_location) : nil - return initial_url(url, joiner) if @initial_param.present? - url = "#{url}#{joiner}#{url_options.to_query}" + result = "#{url}#{joiner}#{url_options.to_query}" + result = initial_url(url, joiner) if @initial_param.present? + result else - url = "#{url}#{joiner}#{url_options.to_query}" + result = "#{url}#{joiner}#{url_options.to_query}" increment_page_counter! - url + result end else url From 57337bdfeca19e0b8f55b91683021c40cfb5b651 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Wed, 14 Feb 2018 13:51:26 +1300 Subject: [PATCH 14/15] . --- spec/supplejack_common/xml/base_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/supplejack_common/xml/base_spec.rb b/spec/supplejack_common/xml/base_spec.rb index 8597bbb..7da46a9 100644 --- a/spec/supplejack_common/xml/base_spec.rb +++ b/spec/supplejack_common/xml/base_spec.rb @@ -23,7 +23,7 @@ end end - describe "next_page_token" do + describe ".next_page_token" do it 'returns the next page token from the document of xml' do klass._document = Nokogiri::XML.parse "token" expect(klass.next_page_token('//NextPageToken')).to eq 'token' From de3c28a80694e44c6f9ca27096622c648af3c999 Mon Sep 17 00:00:00 2001 From: Oliver Stigley Date: Thu, 15 Feb 2018 08:28:55 +1300 Subject: [PATCH 15/15] change tokenised to token --- lib/supplejack_common/paginated_collection.rb | 2 +- spec/supplejack_common/paginated_collection_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/supplejack_common/paginated_collection.rb b/lib/supplejack_common/paginated_collection.rb index d8f4a0f..b6346bc 100644 --- a/lib/supplejack_common/paginated_collection.rb +++ b/lib/supplejack_common/paginated_collection.rb @@ -123,7 +123,7 @@ def paginated? end def tokenised? - @type == 'tokenised' + @type == 'token' end def yield_from_records(&block) diff --git a/spec/supplejack_common/paginated_collection_spec.rb b/spec/supplejack_common/paginated_collection_spec.rb index 5fe2aa2..fffaffb 100644 --- a/spec/supplejack_common/paginated_collection_spec.rb +++ b/spec/supplejack_common/paginated_collection_spec.rb @@ -101,7 +101,7 @@ context "tokenised pagination" do let(:params) { { page_parameter: "page-parameter", - type: "tokenised", + type: "token", per_page_parameter: "per_page", per_page: 5, next_page_token_location: "next_page_token", @@ -122,7 +122,7 @@ context "with initial parameter" do let(:params) { { page_parameter: "page-parameter", - type: "tokenised", + type: "token", initial_param: 'initial-paramater=true' }} let(:collection) { klass.new(SupplejackCommon::Base, params, {limit: 1}) }