From cb89ff8b67fb447213586fe2b1e9dd7e7fb99d36 Mon Sep 17 00:00:00 2001 From: Thomas Walpole Date: Tue, 27 Nov 2018 17:52:35 -0800 Subject: [PATCH] Move css/xpath attribute requirement generation into the builders --- lib/capybara/queries/selector_query.rb | 34 ++----- lib/capybara/selector.rb | 6 +- lib/capybara/selector/builders/css_builder.rb | 37 ++++--- .../selector/builders/xpath_builder.rb | 19 ++-- spec/css_builder_spec.rb | 99 +++++++++++++++++++ spec/xpath_builder_spec.rb | 91 +++++++++++++++++ 6 files changed, 237 insertions(+), 49 deletions(-) create mode 100644 spec/css_builder_spec.rb create mode 100644 spec/xpath_builder_spec.rb diff --git a/lib/capybara/queries/selector_query.rb b/lib/capybara/queries/selector_query.rb index a308aa0f..dd038ee0 100644 --- a/lib/capybara/queries/selector_query.rb +++ b/lib/capybara/queries/selector_query.rb @@ -91,11 +91,11 @@ module Capybara exact = exact? if exact.nil? expr = apply_expression_filters(@expression) expr = exact ? expr.to_xpath(:exact) : expr.to_s if expr.respond_to?(:to_xpath) - filtered_xpath(expr) + filtered_expression(expr) end def css - filtered_css(apply_expression_filters(@expression)) + filtered_expression(apply_expression_filters(@expression)) end # @api private @@ -233,23 +233,11 @@ module Capybara raise ArgumentError, "invalid keys #{invalid_names}, should be one of #{valid_names}" end - def filtered_xpath(expr) - expr = "(#{expr})[#{conditions_from_id}]" if use_default_id_filter? - expr = "(#{expr})[#{conditions_from_classes}]" if use_default_class_filter? - expr - end - - def filtered_css(expr) - id_conditions = conditions_from_id if use_default_id_filter? - id_conditions = [''] unless id_conditions&.any? - - class_conditions = conditions_from_classes if use_default_class_filter? - class_conditions = [''] unless class_conditions&.any? - - conditions = id_conditions.product(class_conditions) - ::Capybara::Selector::CSS.split(expr).map do |sel| - conditions.map { |(id_cond, class_cond)| sel + id_cond + class_cond }.join(', ') - end.join(', ') + def filtered_expression(expr) + conditions = {} + conditions[:id] = options[:id] if use_default_id_filter? + conditions[:class] = options[:class] if use_default_class_filter? + builder.add_attribute_conditions(expr, conditions) end def use_default_id_filter? @@ -260,14 +248,6 @@ module Capybara options.key?(:class) && !custom_keys.include?(:class) end - def conditions_from_classes - builder.class_conditions(options[:class]) - end - - def conditions_from_id - builder.id_conditions(options[:id]) - end - def apply_expression_filters(expression) unapplied_options = options.keys - valid_keys expression_filters.inject(expression) do |expr, (name, ef)| diff --git a/lib/capybara/selector.rb b/lib/capybara/selector.rb index 0b4ae611..30e8028e 100644 --- a/lib/capybara/selector.rb +++ b/lib/capybara/selector.rb @@ -34,7 +34,7 @@ Capybara.add_selector(:css) do end Capybara.add_selector(:id) do - xpath { |id| XPath.descendant[builder.id_conditions(id)] } + xpath { |id| builder.add_attribute_conditions(XPath.descendant, id: id) } locator_filter { |node, id| id.is_a?(Regexp) ? node[:id] =~ id : true } end @@ -119,7 +119,7 @@ Capybara.add_selector(:link) do end expression_filter(:download, valid_values: [true, false, String]) do |expr, download| - expr[builder.attribute_conditions(download: download)] + builder.add_attribute_conditions(expr, download: download) end describe_expression_filters do |**options| @@ -489,7 +489,7 @@ Capybara.add_selector(:element) do end expression_filter(:attributes, matcher: /.+/) do |xpath, name, val| - xpath[builder.attribute_conditions(name => val)] + builder.add_attribute_conditions(xpath, name => val) end node_filter(:attributes, matcher: /.+/) do |node, name, val| diff --git a/lib/capybara/selector/builders/css_builder.rb b/lib/capybara/selector/builders/css_builder.rb index 06d65f10..73104a61 100644 --- a/lib/capybara/selector/builders/css_builder.rb +++ b/lib/capybara/selector/builders/css_builder.rb @@ -30,6 +30,30 @@ module Capybara end.join end + def add_attribute_conditions(selector, **attributes) + attributes.inject(selector) do |css, (name, value)| + conditions = if name == :class + class_conditions(value) + elsif value.is_a? Regexp + Selector::RegexpDisassembler.new(value).alternated_substrings.map do |strs| + strs.map do |str| + "[#{name}*='#{str}'#{' i' if value.casefold?}]" + end.join + end + else + [attribute_conditions(name => value)] + end + + ::Capybara::Selector::CSS.split(css).map do |sel| + next sel if conditions.empty? + + conditions.map { |cond| sel + cond }.join(', ') + end.join(', ') + end + end + + private + def class_conditions(classes) case classes when XPath::Expression @@ -46,19 +70,6 @@ module Capybara cls[true].to_a.map { |cl| ":not(.#{Capybara::Selector::CSS.escape(cl.slice(1..-1))})" }).join] end end - - def id_conditions(id) - case id - when Regexp - Selector::RegexpDisassembler.new(id).alternated_substrings.map do |id_strs| - id_strs.map do |str| - "[id*='#{str}'#{' i' if id.casefold?}]" - end.join - end - else - [attribute_conditions(id: id)] - end - end end end end diff --git a/lib/capybara/selector/builders/xpath_builder.rb b/lib/capybara/selector/builders/xpath_builder.rb index bf10e3a3..88e8d71d 100644 --- a/lib/capybara/selector/builders/xpath_builder.rb +++ b/lib/capybara/selector/builders/xpath_builder.rb @@ -24,6 +24,19 @@ module Capybara end.reduce(:&) end + def add_attribute_conditions(xpath, **conditions) + conditions.inject(xpath) do |xp, (name, value)| + conditions = name == :class ? class_conditions(value) : attribute_conditions(name => value) + if xp.is_a? XPath::Expression + xp[conditions] + else + "(#{xp})[#{conditions}]" + end + end + end + + private + def class_conditions(classes) case classes when XPath::Expression, Regexp @@ -39,12 +52,6 @@ module Capybara end end - def id_conditions(id) - attribute_conditions(id: id) - end - - private - def regexp_to_xpath_conditions(regexp) condition = XPath.current condition = condition.uppercase if regexp.casefold? diff --git a/spec/css_builder_spec.rb b/spec/css_builder_spec.rb new file mode 100644 index 00000000..d6aed792 --- /dev/null +++ b/spec/css_builder_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Capybara::Selector::CSSBuilder do + let :builder do + ::Capybara::Selector::CSSBuilder + end + + context 'add_attribute_conditions' do + it 'adds a single string condition to a single selector' do + css = 'div' + selector = builder.add_attribute_conditions(css, random: 'abc') + expect(selector).to eq %(div[random='abc']) + end + + it 'adds multiple string conditions to a single selector' do + css = 'div' + selector = builder.add_attribute_conditions(css, random: 'abc', other: 'def') + expect(selector).to eq %(div[random='abc'][other='def']) + end + + it 'adds a single string condition to a multiple selector' do + css = 'div, ul' + selector = builder.add_attribute_conditions(css, random: 'abc') + expect(selector).to eq %(div[random='abc'], ul[random='abc']) + end + + it 'adds multiple string conditions to a multiple selector' do + css = 'div, ul' + selector = builder.add_attribute_conditions(css, random: 'abc', other: 'def') + expect(selector).to eq %(div[random='abc'][other='def'], ul[random='abc'][other='def']) + end + + it 'adds simple regexp conditions to a single selector' do + css = 'div' + selector = builder.add_attribute_conditions(css, random: /abc/, other: /def/) + expect(selector).to eq %(div[random*='abc'][other*='def']) + end + + it 'adds wildcard regexp conditions to a single selector' do + css = 'div' + selector = builder.add_attribute_conditions(css, random: /abc.*def/, other: /def.*ghi/) + expect(selector).to eq %(div[random*='abc'][random*='def'][other*='def'][other*='ghi']) + end + + it 'adds alternated regexp conditions to a single selector' do + css = 'div' + selector = builder.add_attribute_conditions(css, random: /abc|def/, other: /def|ghi/) + expect(selector).to eq %(div[random*='abc'][other*='def'], div[random*='abc'][other*='ghi'], div[random*='def'][other*='def'], div[random*='def'][other*='ghi']) + end + + it 'adds alternated regexp conditions to a multiple selector' do + css = 'div,ul' + selector = builder.add_attribute_conditions(css, other: /def.*ghi|jkl/) + expect(selector).to eq %(div[other*='def'][other*='ghi'], div[other*='jkl'], ul[other*='def'][other*='ghi'], ul[other*='jkl']) + end + + it "returns original selector when regexp can't be substringed" do + css = 'div' + selector = builder.add_attribute_conditions(css, other: /.+/) + expect(selector).to eq 'div' + end + + context ':class' do + it 'handles string with CSS .' do + css = 'a' + selector = builder.add_attribute_conditions(css, class: 'my_class') + expect(selector).to eq 'a.my_class' + end + + it 'handles negated string with CSS .' do + css = 'a' + selector = builder.add_attribute_conditions(css, class: '!my_class') + expect(selector).to eq 'a:not(.my_class)' + end + + it 'handles array of string with CSS .' do + css = 'a' + selector = builder.add_attribute_conditions(css, class: %w[my_class my_other_class]) + expect(selector).to eq 'a.my_class.my_other_class' + end + + it 'handles array of string with CSS . when negated included' do + css = 'a' + selector = builder.add_attribute_conditions(css, class: %w[my_class !my_other_class]) + expect(selector).to eq 'a.my_class:not(.my_other_class)' + end + end + + context ':id' do + it 'handles string with CSS #' do + css = 'ul' + selector = builder.add_attribute_conditions(css, id: 'my_id') + expect(selector).to eq 'ul#my_id' + end + end + end +end diff --git a/spec/xpath_builder_spec.rb b/spec/xpath_builder_spec.rb new file mode 100644 index 00000000..43c148f5 --- /dev/null +++ b/spec/xpath_builder_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Capybara::Selector::XPathBuilder do + let :builder do + ::Capybara::Selector::XPathBuilder + end + + context 'add_attribute_conditions' do + it 'adds a single string condition to a single selector' do + xpath = './/div' + selector = builder.add_attribute_conditions(xpath, random: 'abc') + expect(selector).to eq %((.//div)[(./@random = 'abc')]) + end + + it 'adds multiple string conditions to a single selector' do + xpath = './/div' + selector = builder.add_attribute_conditions(xpath, random: 'abc', other: 'def') + expect(selector).to eq %(((.//div)[(./@random = 'abc')])[(./@other = 'def')]) + end + + it 'adds a single string condition to a multiple selector' do + xpath = XPath.descendant(:div, :ul) + selector = builder.add_attribute_conditions(xpath, random: 'abc') + expect(selector.to_s).to eq xpath[XPath.attr(:random) == 'abc'].to_s + end + + it 'adds multiple string conditions to a multiple selector' do + xpath = XPath.descendant(:div, :ul) + selector = builder.add_attribute_conditions(xpath, random: 'abc', other: 'def') + expect(selector.to_s).to eq %(.//*[self::div | self::ul][(./@random = 'abc')][(./@other = 'def')]) + end + + it 'adds simple regexp conditions to a single selector' do + xpath = XPath.descendant(:div) + selector = builder.add_attribute_conditions(xpath, random: /abc/, other: /def/) + expect(selector.to_s).to eq %(.//div[./@random[contains(., 'abc')]][./@other[contains(., 'def')]]) + end + + it 'adds wildcard regexp conditions to a single selector' do + xpath = './/div' + selector = builder.add_attribute_conditions(xpath, random: /abc.*def/, other: /def.*ghi/) + expect(selector).to eq %(((.//div)[./@random[(contains(., 'abc') and contains(., 'def'))]])[./@other[(contains(., 'def') and contains(., 'ghi'))]]) + end + + it 'adds alternated regexp conditions to a single selector' do + xpath = XPath.descendant(:div) + selector = builder.add_attribute_conditions(xpath, random: /abc|def/, other: /def|ghi/) + expect(selector.to_s).to eq %(.//div[./@random[(contains(., 'abc') or contains(., 'def'))]][./@other[(contains(., 'def') or contains(., 'ghi'))]]) + end + + it 'adds alternated regexp conditions to a multiple selector' do + xpath = XPath.descendant(:div, :ul) + selector = builder.add_attribute_conditions(xpath, other: /def.*ghi|jkl/) + expect(selector.to_s).to eq %(.//*[self::div | self::ul][./@other[((contains(., 'def') and contains(., 'ghi')) or contains(., 'jkl'))]]) + end + + it "returns original selector when regexp can't be substringed" do + xpath = './/div' + selector = builder.add_attribute_conditions(xpath, other: /.+/) + expect(selector).to eq '(.//div)[./@other]' + end + + context ':class' do + it 'handles string' do + xpath = './/a' + selector = builder.add_attribute_conditions(xpath, class: 'my_class') + expect(selector).to eq %((.//a)[contains(concat(' ', normalize-space(./@class), ' '), ' my_class ')]) + end + + it 'handles negated strings' do + xpath = XPath.descendant(:a) + selector = builder.add_attribute_conditions(xpath, class: '!my_class') + expect(selector.to_s).to eq xpath[!XPath.attr(:class).contains_word('my_class')].to_s + end + + it 'handles array of strings' do + xpath = './/a' + selector = builder.add_attribute_conditions(xpath, class: %w[my_class my_other_class]) + expect(selector).to eq %((.//a)[(contains(concat(' ', normalize-space(./@class), ' '), ' my_class ') and contains(concat(' ', normalize-space(./@class), ' '), ' my_other_class '))]) + end + + it 'handles array of string when negated included' do + xpath = XPath.descendant(:a) + selector = builder.add_attribute_conditions(xpath, class: %w[my_class !my_other_class]) + expect(selector.to_s).to eq xpath[XPath.attr(:class).contains_word('my_class') & !XPath.attr(:class).contains_word('my_other_class')].to_s + end + end + end +end