refactor the extends and inherited_block tags, uncouple them from the persistence layer + add specs

did committed Feb 03, 2015

commit a005442454050bc4d41091690700010004bca10e

Gemfile +2 -1

@@	@@ -6,7 +6,8 @@ group :development do
	# gem 'locomotivecms_common', '~> 0.0.2', path: '../common'
	# gem 'locomotivecms_models', '~> 0.0.1', path: '../models'
	# gem 'locomotivecms_models', '0.0.1.pre.alpha'
-	gem 'thin'
+	# gem 'locomotivecms-liquid', path: '/Users/didier/Documents/LocomotiveCMS/gems/liquid'
+	# gem 'thin'
	end

	group :test do

Gemfile.lock +4 -11

@@	@@ -11,7 +11,7 @@ PATH
	httparty (~> 0.13.3)
	kaminari (~> 0.16.2)
	kramdown (~> 1.5.0)
-	locomotivecms-solid (~> 4.0.0.alpha)
+	locomotivecms-solid (~> 4.0.0.alpha1)
	locomotivecms_common (~> 0.0.2)
	mimetype-fu (~> 0.1.2)
	moneta (~> 0.8.0)
@@	@@ -71,7 +71,6 @@ GEM
	simplecov (~> 0.9.1)
	term-ansicolor (~> 1.3)
	thor (~> 0.19.1)
-	daemons (1.1.9)
	diff-lcs (1.2.5)
	docile (1.1.5)
	dragonfly (1.0.7)
@@	@@ -79,7 +78,6 @@ GEM
	multi_json (~> 1.0)
	rack
	erubis (2.7.0)
-	eventmachine (1.0.4)
	execjs (2.2.2)
	ffi (1.9.6)
	haml (4.0.6)
@@	@@ -101,9 +99,9 @@ GEM
	actionpack (>= 3.0.0)
	activesupport (>= 3.0.0)
	kramdown (1.5.0)
-	locomotivecms-liquid (4.0.0.alpha)
-	locomotivecms-solid (4.0.0.alpha)
-	locomotivecms-liquid (= 4.0.0.alpha)
+	locomotivecms-liquid (4.0.0.alpha1)
+	locomotivecms-solid (4.0.0.alpha1)
+	locomotivecms-liquid (= 4.0.0.alpha1)
	locomotivecms_common (0.0.2)
	colorize
	loofah (2.0.1)
@@	@@ -175,10 +173,6 @@ GEM
	tilt (~> 1.1)
	term-ansicolor (1.3.0)
	tins (~> 1.0)
-	thin (1.6.3)
-	daemons (~> 1.0, >= 1.0.9)
-	eventmachine (~> 1.0)
-	rack (~> 1.0)
	thor (0.19.1)
	thread_safe (0.3.4)
	tilt (1.4.1)
@@	@@ -198,4 +192,3 @@ DEPENDENCIES
	pry
	rake (~> 10.4.2)
	rspec (~> 3.1.0)
-	thin

locomotive/steam/liquid/tags/extends.rb b/lib/locomotive/steam/liquid/tags/extends.rb +40 -28

@@	@@ -4,40 +4,20 @@ module Locomotive
	module Tags
	class Extends < ::Liquid::Extends

-	def prepare_parsing
-	super
-
-	parent_page = @context[:parent_page]
-
-	@context[:page].merge_editable_elements_from_page(parent_page)
-
-	@context[:snippets] = parent_page.snippet_dependencies
-	@context[:templates] = ([*parent_page.template_dependencies] + [parent_page.id]).compact
-	end
-
	private

	def parse_parent_template
-	if @template_name == 'parent'
-	@context[:parent_page] = @context[:cached_parent] \|\| @context[:page].parent
-	else
-	locale = ::Mongoid::Fields::I18n.locale
+	parent = options[:parent_finder].find(options[:page], @template_name)

-	@context[:parent_page] = @context[:cached_pages].try(:[], @template_name) \|\|
-	@context[:site].pages.where("fullpath.#{locale}" => @template_name).first
-	end
+	# no need to go further if the parent does not exist
+	raise PageNotFound.new("Page with fullpath '#{@template_name}' was not found") if parent.nil?

-	raise PageNotFound.new("Page with fullpath '#{@template_name}' was not found") if @context[:parent_page].nil?
-
-	# be sure to work with a copy of the parent template otherwise there will be conflicts
-	parent_template = @context[:parent_page].template.try(:clone)
-
-	raise PageNotTranslated.new("Page with fullpath '#{@template_name}' was not translated") if parent_template.nil?
-
-	# force the page to restore the original version of its template (from the serialized version)
-	@context[:parent_page].instance_variable_set(:@template, nil)
+	if listener = options[:events_listener]
+	listener.emit(:extends, page: options[:page], parent: parent)
+	end

-	parent_template
+	# the source has already been parsed before
+	parent.template \|\| ::Liquid::Template.parse(parent.source, options.merge(page: parent))
	end

	end
@@	@@ -47,3 +27,35 @@ module Locomotive
	end
	end
	end
+
+	# def prepare_parsing
+	# super
+
+	# parent_page = @context[:parent_page]
+
+	# @context[:page].merge_editable_elements_from_page(parent_page)
+
+	# @context[:snippets] = parent_page.snippet_dependencies
+	# @context[:templates] = ([*parent_page.template_dependencies] + [parent_page.id]).compact
+	# end
+
+	# if @template_name == 'parent'
+	# @context[:parent_page] = @context[:cached_parent] \|\| @context[:page].parent
+	# else
+	# locale = ::Mongoid::Fields::I18n.locale
+
+	# @context[:parent_page] = @context[:cached_pages].try(:[], @template_name) \|\|
+	# @context[:site].pages.where("fullpath.#{locale}" => @template_name).first
+	# end
+
+	# raise PageNotFound.new("Page with fullpath '#{@template_name}' was not found") if @context[:parent_page].nil?
+
+	# # be sure to work with a copy of the parent template otherwise there will be conflicts
+	# parent_template = @context[:parent_page].template.try(:clone)
+
+	# raise PageNotTranslated.new("Page with fullpath '#{@template_name}' was not translated") if parent_template.nil?
+
+	# # force the page to restore the original version of its template (from the serialized version)
+	# @context[:parent_page].instance_variable_set(:@template, nil)
+
+	# parent_template

locomotive/steam/liquid/tags/fetch_page.rb b/lib/locomotive/steam/liquid/tags/fetch_page.rb +1 -1

@@	@@ -25,7 +25,7 @@ module Locomotive
	end

	def render(context)
-	page = context.registers[:repositories].page.find_by_handle(@handle)
+	page = context.registers[:repositories].page.by_handle(@handle)
	context.scopes.last[@var] = page
	''
	end

locomotive/steam/liquid/tags/inherited_block.rb b/lib/locomotive/steam/liquid/tags/inherited_block.rb +15 -9

@@	@@ -4,26 +4,32 @@ module Locomotive
	module Tags
	class InheritedBlock < ::Liquid::InheritedBlock

-	def end_tag
-	super
-
-	if !self.contains_super?(@nodelist) # then disable all editable_elements coming from the parent block too and not used
-	@context[:page].disable_parent_editable_elements(@name) unless @context[:page].nil?
+	def parse(tokens)
+	super.tap do
+	if listener = options[:events_listener]
+	listener.emit(:inherited_block, page: options[:page], name: @name, found_super: self.contains_super?(nodelist))
+	end
	end
	end

	protected

-	def contains_super?(nodelist)
-	nodelist.any? do \|node\|
-	if node.is_a?(::Liquid::Variable) && node.name == 'block.super'
+	def contains_super?(nodes)
+	nodes.any? do \|node\|
+	if is_node_block_super?(node)
	true
-	elsif node.respond_to?(:nodelist) && !node.nodelist.nil? && !node.is_a?(Locomotive::Liquid::Tags::InheritedBlock)
+	elsif node.respond_to?(:nodelist) && !node.nodelist.nil? && !node.is_a?(Locomotive::Steam::Liquid::Tags::InheritedBlock)
	contains_super?(node.nodelist)
	end
	end
	end

+	def is_node_block_super?(node)
+	return unless node.is_a?(::Liquid::Variable)
+
+	node.raw.strip == 'block.super'
+	end
+
	end

	::Liquid::Template.register_tag('block', InheritedBlock)

locomotive/steam/repositories/page.rb b/lib/locomotive/steam/repositories/page.rb +9 -1

@@	@@ -4,10 +4,18 @@ module Locomotive

	class Page < Struct.new(:site)

-	def find_by_handle(handle)
+	def by_handle(handle)
	site.pages.where(handle: handle).first
	end

+	def parent_of(page)
+	page.parent
+	end
+
+	def by_fullpath(path)
+	site.pages.where(fullpath: path).first
+	end
+
	end

	end

locomotive/steam/services/parent_finder.rb b/lib/locomotive/steam/services/parent_finder.rb +27 -0

@@	@@ -0,0 +1,27 @@
+	module Locomotive
+	module Steam
+	module Services
+
+	class ParentFinder < Struct.new(:site)
+
+	include Morphine
+
+	register :repository do
+	Repositories::Page.new(site)
+	end
+
+	def find(page, fullpath)
+	return nil if fullpath.blank?
+
+	if fullpath.strip == 'parent'
+	repository.parent_of(page)
+	else
+	repository.by_fullpath(fullpath)
+	end
+	end
+
+	end
+
+	end
+	end
+	end

locomotivecms_steam.gemspec +1 -4

@@	@@ -40,11 +40,8 @@ Gem::Specification.new do \|spec\|
	spec.add_dependency 'haml', '~> 4.0.6'
	spec.add_dependency 'mimetype-fu', '~> 0.1.2'

-
-
-
	# spec.add_dependency 'locomotivecms_models', '~> 0.0.1.pre.alpha'
-	spec.add_dependency 'locomotivecms-solid', '~> 4.0.0.alpha'
+	spec.add_dependency 'locomotivecms-solid', '~> 4.0.0.alpha1'
	spec.add_dependency 'locomotivecms_common', '~> 0.0.2'

	# spec.required_ruby_version = '~> 2.0'

spec/support/liquid.rb +18 -0

@@	@@ -3,3 +3,21 @@ def render_template(template, context = nil)
	context.exception_handler = ->(e) { true }
	::Liquid::Template.parse(template).render(context)
	end
+
+	def parse_template(template, options = nil)
+	::Liquid::Template.parse(template, options \|\| {})
+	end
+
+	module Liquid
+	class SimpleEventsListener
+	def emit(name, options = {})
+	(@stack \|\|= []) << [name, options]
+	end
+	def event_names
+	@stack.map { \|(name, _)\| name }
+	end
+	def events
+	@stack
+	end
+	end
+	end

spec/unit/liquid/tags/extends_spec.rb +51 -0

@@	@@ -0,0 +1,51 @@
+	require 'spec_helper'
+
+	describe Locomotive::Steam::Liquid::Tags::Extends do
+
+	let(:source) { '{% extends parent %} ' }
+	let(:page) { instance_double('Page', title: 'About us') }
+	let(:listener) { Liquid::SimpleEventsListener.new }
+	let(:finder) { Locomotive::Steam::Services::ParentFinder.new(nil) }
+	let(:options) { { events_listener: listener, parent_finder: finder, page: page } }
+
+	before do
+	allow(finder.repository).to receive(:parent_of).and_return(parent)
+	end
+
+	describe 'no parent page found' do
+
+	let(:parent) { nil }
+	let(:template) { parse_template(source, options) }
+
+	it { expect { template }.to raise_exception Locomotive::Steam::Liquid::PageNotFound }
+
+	end
+
+	describe 'parent page exists' do
+
+	let!(:template) { parse_template(source, options) }
+
+	describe 'parent template already parsed' do
+
+	let(:parent_template) { parse_template('Hello world') }
+	let(:parent) { instance_double('Index', template: parent_template) }
+
+	it { expect(listener.event_names.first).to eq :extends }
+	it { expect(template.render).to eq 'Hello world' }
+	it { expect(options[:page]).to eq page }
+
+	end
+
+	describe 'parent template not parsed yet' do
+
+	let(:parent) { instance_double('Index', source: 'Hello world!', template: nil) }
+
+	it { expect(listener.event_names.first).to eq :extends }
+	it { expect(template.render).to eq 'Hello world!' }
+	it { expect(options[:page]).to eq page }
+
+	end
+
+	end
+
+	end

spec/unit/liquid/tags/fetch_page_spec.rb +1 -1

@@	@@ -8,7 +8,7 @@ describe Locomotive::Steam::Liquid::Tags::FetchPage do
	let(:context) { ::Liquid::Context.new(assigns, {}, { repositories: repositories }) }

	let(:page) { instance_double('Page', to_liquid: { 'title' => 'About Us' }) }
-	before { allow(repositories.page).to receive(:find_by_handle).and_return(page) }
+	before { allow(repositories.page).to receive(:by_handle).and_return(page) }

	subject { render_template(template, context) }

spec/unit/liquid/tags/inherited_block_spec.rb +33 -0

@@	@@ -0,0 +1,33 @@
+	require 'spec_helper'
+
+	describe Locomotive::Steam::Liquid::Tags::InheritedBlock do
+
+	let(:parent_source) { 'My product: {% block product %}Random{% endblock %}' }
+	let(:parent) { instance_double('Index', source: parent_source, template: nil) }
+	let(:source) { '{% extends parent %}{% block product %}Skis{% endblock %}' }
+	let(:page) { instance_double('Page') }
+
+	let(:listener) { Liquid::SimpleEventsListener.new }
+	let(:finder) { instance_double('Finder', find: parent) }
+	let(:options) { { page: page, events_listener: listener, parent_finder: finder } }
+
+	let!(:template) { parse_template(source, options) }
+
+	describe 'without a super block' do
+
+	it { expect(listener.events.size).to eq 3 }
+	it { expect(listener.events.first.last[:found_super]).to eq false }
+	it { expect(template.render).to eq 'My product: Skis' }
+
+	end
+
+	describe 'with a super block' do
+
+	let(:source) { '{% extends parent %}{% block product %}Skis (previous: {{ block.super }}){% endblock %}' }
+
+	it { expect(listener.events.first.last[:found_super]).to eq true }
+	it { expect(template.render).to eq 'My product: Skis (previous: Random)' }
+
+	end
+
+	end

spec/unit/services/parent_finder_spec.rb +40 -0

@@	@@ -0,0 +1,40 @@
+	require 'spec_helper'
+
+	describe Locomotive::Steam::Services::ParentFinder do
+
+	let(:service) { Locomotive::Steam::Services::ParentFinder.new(nil) }
+	let(:repository) { service.repository }
+
+	describe '#find' do
+
+	let(:name) { '' }
+	let(:another_page) { instance_double('Index', title: 'Index') }
+	let(:page) { instance_double('AboutUs', title: 'About us') }
+
+	subject { service.find(page, name) }
+
+	it { is_expected.to eq nil }
+
+	describe 'using the parent keyword' do
+
+	let(:name) { 'parent' }
+
+	before { expect(repository).to receive(:parent_of).and_return(another_page) }
+
+	it { is_expected.to eq another_page }
+
+	end
+
+	describe 'using the fullpath' do
+
+	let(:name) { 'index' }
+
+	before { expect(repository).to receive(:by_fullpath).with('index').and_return(another_page) }
+
+	it { is_expected.to eq another_page }
+
+	end
+
+	end
+
+	end