Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable watch to prevent race condition between threads calling site.process #320

Merged
merged 23 commits into from
Mar 15, 2017
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
fd83a67
When updating a document or page, read item by itself
parkr Mar 9, 2017
1203e14
Use ensure_* helper methods and use this more robust method as a fall…
parkr Mar 9, 2017
9e7b78d
Use accurate method names; add the || for the page server back.
parkr Mar 11, 2017
efd6ede
Merge branch 'master' into prevent-race-condition-on-change
mertkahyaoglu Mar 11, 2017
4ce5082
centralize path manipulation methods in server.rb
benbalter Mar 13, 2017
c43c34e
slightly less abstraction
benbalter Mar 13, 2017
0c170bd
DRY up removing the site source
benbalter Mar 13, 2017
82aba50
move path helper methods to its own module
benbalter Mar 13, 2017
a573bb5
correct rubocop offenses;
benbalter Mar 13, 2017
87f8e2b
centralize file manipulation methods in file helper
benbalter Mar 14, 2017
9514901
autoload ALL THE THINGS!
benbalter Mar 14, 2017
2bf85ae
add some missing tests
benbalter Mar 14, 2017
3652c41
use FileHelper and PathHelper for DataFile methods
benbalter Mar 14, 2017
f4d94ab
add entries url to collection hash
benbalter Mar 14, 2017
1d5a49b
directory_path should be private
benbalter Mar 14, 2017
5a87555
actually, actually disable watch
benbalter Mar 14, 2017
e26934f
Use @ashmaroli's approach to monkey patching the build command to dis…
benbalter Mar 15, 2017
7d086f2
stop server before starting just in case
benbalter Mar 15, 2017
c8fab6c
-f, not -9
benbalter Mar 15, 2017
e1af381
Merge branch 'master' into prevent-race-condition-on-change
benbalter Mar 15, 2017
28c4f9c
revert changes to lib/jekyll/commands/serve.rb
benbalter Mar 15, 2017
1f8c8c2
test --watch with this plugin
ashmaroli Mar 15, 2017
bce59f7
Merge pull request #324 from ashmaroli/test-watch-admin
benbalter Mar 15, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 79 additions & 6 deletions lib/jekyll-admin/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,72 @@ def render_404
end

def request_payload
@request_payload ||= begin
request.body.rewind
JSON.parse request.body.read
end
@request_payload ||= if request_body.to_s.empty?
{}
else
JSON.parse(request_body)
end
end

def base_url
"#{request.scheme}://#{request.host_with_port}"
end

def sanitized_path(questionable_path)
Jekyll.sanitized_path JekyllAdmin.site.source, questionable_path
def sanitized_path(path)
path = path.to_s.gsub(%r!\A#{Regexp.escape(JekyllAdmin.site.source)}!, "")
Jekyll.sanitized_path JekyllAdmin.site.source, path
end

# Returns the sanitized path relative to the site source
def sanitized_relative_path(path)
path = sanitized_path(path)
path.sub(%r!\A#{Regexp.escape(JekyllAdmin.site.source)}!, "")
end

def filename
"#{params["path"]}.#{params["ext"]}"
end

# Returns the sanitized absolute path to the requested object
def path
sanitized_path File.join(directory_path, filename)
end

# Returns the sanitized relative path to the requested object
def relative_path
sanitized_relative_path path
end

# Returns the sanitized absolute path to write the requested object
def write_path
if renamed?
sanitized_path request_payload["path"]
else
path
end
end
alias_method :request_path, :write_path

# Returns the sanitized relative path to write the requested object
def relative_write_path
sanitized_relative_path write_path
end

def renamed?
return false unless request_payload["path"]
ensure_leading_slash(request_payload["path"]) != relative_path
end

def directory_path
if namespace == "collections"
sanitized_path File.join(collection.relative_directory, params["splat"].first)
else
sanitized_path params["splat"].first
end
end

def ensure_directory
render_404 unless Dir.exist?(directory_path)
end

def front_matter
Expand Down Expand Up @@ -82,5 +136,24 @@ def delete_file(path)
File.delete sanitized_path(path)
site.process
end

private

def request_body
@request_body ||= begin
request.body.rewind
request.body.read
end
end

def namespace
namespace = request.fullpath.split("/")[1].downcase
namespace if ROUTES.include?(namespace)
end

def ensure_leading_slash(input)
return input if input.nil? || input.empty? || input.start_with?("/")
"/#{input}"
end
end
end
46 changes: 9 additions & 37 deletions lib/jekyll-admin/server/collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,15 @@ class Server < Sinatra::Base

put "/:collection_id/*?/?:path.:ext" do
ensure_collection
write_path = document_path
if request_payload["path"] && request_payload["path"] != relative_document_path
delete_file document_path
write_path = request_path
end

write_file(write_path, document_body)
updated_document = collection.docs.find { |d| d.path == write_path }
render_404 if updated_document.nil?
json updated_document.to_api(:include_content => true)
delete_file path if renamed?
write_file write_path, document_body
ensure_document
json document.to_api(:include_content => true)
end

delete "/:collection_id/*?/?:path.:ext" do
ensure_document
delete_file document_path
delete_file path
content_type :json
status 200
halt
Expand All @@ -49,44 +43,22 @@ def collection
collection[1] if collection
end

def request_path
sanitized_path request_payload["path"]
end

def filename
"#{params["path"]}.#{params["ext"]}"
end

def document_id
path = "#{params["splat"].first}/#{filename}"
path.gsub(%r!(\d{4})/(\d{2})/(\d{2})/(.*)!, '\1-\2-\3-\4')
end

def document_path
sanitized_path File.join(collection.relative_directory, document_id)
end

def relative_document_path
if params["splat"].first.empty?
File.join(collection.relative_directory, filename)
else
relative_to_collection = File.join(params["splat"].first, filename)
File.join(collection.relative_directory, relative_to_collection)
end
end

def document
collection.docs.find { |d| d.path == document_path }
collection.docs.find { |d| d.path == path } ||
Jekyll::Document.new(write_path, {
:collection => collection, :site => site,
}).tap(&:read)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really 🆒 You da boss 😎

end

def directory_docs
collection.docs.find_all { |d| File.dirname(d.path) == directory_path }
end

def directory_path
sanitized_path File.join(collection.relative_directory, params["splat"].first)
end

def ensure_collection
render_404 if collection.nil?
end
Expand Down
48 changes: 11 additions & 37 deletions lib/jekyll-admin/server/page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,15 @@ class Server < Sinatra::Base

put "/*?/?:path.:ext" do
ensure_html_content
write_path = relative_page_path
if request_payload["path"] && request_payload["path"] != relative_page_path
delete_file page_path
write_path = request_payload["path"]
end

write_file(write_path, page_body)
updated_page = pages.find { |p| p.path == write_path }
render_404 if updated_page.nil?
json updated_page.to_api(:include_content => true)
delete_file path if renamed?
write_file write_path, page_body
ensure_page
json page.to_api(:include_content => true)
end

delete "/*?/?:path.:ext" do
ensure_page
delete_file page_path
delete_file path
content_type :json
status 200
halt
Expand All @@ -52,14 +46,6 @@ def html_content?
page.html?
end

def request_path
sanitized_path request_payload["path"]
end

def filename
"#{params["path"]}.#{params["ext"]}"
end

def pages
site.pages.select(&:html?)
end
Expand All @@ -75,25 +61,13 @@ def directory_paths
pages.map { |p| File.dirname(p.path).split("/")[0] }.uniq
end

def page_path
File.join(directory_path, filename)
end

def relative_page_path
return filename if params["splat"].first.empty?
File.join(params["splat"].first, filename)
end

def page
site.pages.find { |p| sanitized_path(p.path) == page_path }
end

def directory_path
sanitized_path params["splat"].first
end

def ensure_directory
render_404 unless Dir.exist?(directory_path)
found = site.pages.find { |p| sanitized_path(p.path) == path }
return found if found
return unless File.file?(write_path)
Jekyll::Page.new(
Copy link
Member

@mertkahyaoglu mertkahyaoglu Mar 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed || here or is this on purpose?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do either.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think he's asking if we want to look to the existing page first then fall back to creating a new one like we do for other content types?

site, site.source, File.dirname(write_path), File.basename(write_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want the full path for dir ?

)
end

def ensure_page
Expand Down
25 changes: 14 additions & 11 deletions lib/jekyll-admin/server/static_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,21 @@ class Server < Sinatra::Base
end

put "/*" do
write_file(static_file_path, static_file_body)
write_file(write_path, static_file_body)
ensure_static_file_exists
json static_file.to_api(:include_content => true)
end

delete "/*" do
ensure_static_file_exists
delete_file static_file_path
delete_file path
content_type :json
status 200
halt
end

private

def static_file_path
if params["splat"]
params["static_file_id"] = params["splat"].first
end
sanitized_path params["static_file_id"]
end

def static_file_body
if !request_payload["raw_content"].to_s.empty?
request_payload["raw_content"].to_s
Expand All @@ -52,13 +45,23 @@ def static_files

def file_list_dir(path) end

# TODO: StaticFile.new needs to be taught about the collection the
# file belongs to, if it belongs to a collection.
def static_file
static_files.find { |f| f.path == static_file_path }
found = static_files.find { |f| f.path == relative_path }
return found if found
return unless File.file?(write_path)
Jekyll::StaticFile.new(
site,
site.source,
File.dirname(relative_write_path),
File.basename(relative_write_path)
)
end

def static_files_for_path
# Joined with / to ensure user can't do partial paths
base_path = File.join(static_file_path, "/")
base_path = File.join(path, "/")
static_files.select do |f|
f.path.start_with? base_path
end
Expand Down
5 changes: 5 additions & 0 deletions spec/fixtures/site/_posts/test/2016-01-02-test2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
foo: bar2
---

test