Skip to content

Commit 8fdfb78

Browse files
authored
Merge pull request #232 from glennsarti/async-diagnostics
(GH-231) Make document validation asynchronous
2 parents df05a14 + e5c99e9 commit 8fdfb78

File tree

7 files changed

+369
-130
lines changed

7 files changed

+369
-130
lines changed

client/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ Check [Keep a Changelog](http://keepachangelog.com/) for recommendations on how
77
## Unreleased
88

99
- ([GH-225](https://github.com/jpogran/puppet-vscode/issues/225)) Readd Local Workspace comand line option
10+
- ([GH-231](https://github.com/jpogran/puppet-vscode/issues/231)) Make Document Validation asynchronous
1011

1112
## 0.9.0 - 2018-02-01
1213

server/lib/puppet-languageserver.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
require 'languageserver/languageserver'
66
require 'puppet-vscode'
77

8-
%w[json_rpc_handler message_router server_capabilities document_validator puppet_parser_helper puppet_helper
8+
%w[json_rpc_handler message_router validation_queue server_capabilities document_validator puppet_parser_helper puppet_helper
99
facter_helper completion_provider hover_provider definition_provider puppet_monkey_patches].each do |lib|
1010
begin
1111
require "puppet-languageserver/#{lib}"

server/lib/puppet-languageserver/message_router.rb

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,42 @@
11
module PuppetLanguageServer
2-
# TODO: Thread/Atomic safe? probably not
32
module DocumentStore
43
@documents = {}
5-
6-
def self.set_document(uri, content)
7-
@documents[uri] = content
4+
@doc_mutex = Mutex.new
5+
6+
def self.set_document(uri, content, doc_version)
7+
@doc_mutex.synchronize do
8+
@documents[uri] = {
9+
:content => content,
10+
:version => doc_version
11+
}
12+
end
813
end
914

1015
def self.remove_document(uri)
11-
@documents[uri] = nil
16+
@doc_mutex.synchronize { @documents[uri] = nil }
1217
end
1318

1419
def self.clear
15-
@documents.clear
20+
@doc_mutex.synchronize { @documents.clear }
21+
end
22+
23+
def self.document(uri, doc_version = nil)
24+
@doc_mutex.synchronize do
25+
return nil if @documents[uri].nil?
26+
return nil unless doc_version.nil? || @documents[uri][:version] == doc_version
27+
@documents[uri][:content].clone
28+
end
1629
end
1730

18-
def self.document(uri)
19-
return nil if @documents[uri].nil?
20-
@documents[uri].clone
31+
def self.document_version(uri)
32+
@doc_mutex.synchronize do
33+
return nil if @documents[uri].nil?
34+
@documents[uri][:version]
35+
end
2136
end
2237

2338
def self.document_uris
24-
@documents.keys.dup
39+
@doc_mutex.synchronize { @documents.keys.dup }
2540
end
2641
end
2742

@@ -280,15 +295,9 @@ def receive_notification(method, params)
280295
PuppetLanguageServer.log_message(:info, 'Received textDocument/didOpen notification.')
281296
file_uri = params['textDocument']['uri']
282297
content = params['textDocument']['text']
283-
documents.set_document(file_uri, content)
284-
case document_type(file_uri)
285-
when :manifest
286-
reply_diagnostics(file_uri, PuppetLanguageServer::DocumentValidator.validate(content, @workspace))
287-
when :epp
288-
reply_diagnostics(file_uri, PuppetLanguageServer::DocumentValidator.validate_epp(content, @workspace))
289-
else
290-
reply_diagnostics(file_uri, [])
291-
end
298+
doc_version = params['textDocument']['version']
299+
documents.set_document(file_uri, content, doc_version)
300+
PuppetLanguageServer::ValidationQueue.enqueue(file_uri, doc_version, @workspace, self)
292301

293302
when 'textDocument/didClose'
294303
PuppetLanguageServer.log_message(:info, 'Received textDocument/didClose notification.')
@@ -299,15 +308,9 @@ def receive_notification(method, params)
299308
PuppetLanguageServer.log_message(:info, 'Received textDocument/didChange notification.')
300309
file_uri = params['textDocument']['uri']
301310
content = params['contentChanges'][0]['text'] # TODO: Bad hardcoding zero
302-
documents.set_document(file_uri, content)
303-
case document_type(file_uri)
304-
when :manifest
305-
reply_diagnostics(file_uri, PuppetLanguageServer::DocumentValidator.validate(content, @workspace))
306-
when :epp
307-
reply_diagnostics(file_uri, PuppetLanguageServer::DocumentValidator.validate_epp(content, @workspace))
308-
else
309-
reply_diagnostics(file_uri, [])
310-
end
311+
doc_version = params['textDocument']['version']
312+
documents.set_document(file_uri, content, doc_version)
313+
PuppetLanguageServer::ValidationQueue.enqueue(file_uri, doc_version, @workspace, self)
311314

312315
when 'textDocument/didSave'
313316
PuppetLanguageServer.log_message(:info, 'Received textDocument/didSave notification.')
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
module PuppetLanguageServer
2+
# Module for enqueing and running document level validation asynchronously
3+
# When adding a document to be validation, it will remove any validation requests for the same
4+
# document in the queue so that only the latest document needs to be processed.
5+
#
6+
# It will also ignore sending back validation results to the client if the document is
7+
# updated during the validation process
8+
module ValidationQueue
9+
@queue = []
10+
@queue_mutex = Mutex.new
11+
@queue_thread = nil
12+
13+
# Enqueue a file to be validated
14+
def self.enqueue(file_uri, doc_version, workspace, connection_object)
15+
document_type = connection_object.document_type(file_uri)
16+
17+
unless %i[manifest epp].include?(document_type)
18+
# Can't validate these types so just emit an empty validation result
19+
connection_object.reply_diagnostics(file_uri, [])
20+
return
21+
end
22+
23+
@queue_mutex.synchronize do
24+
@queue.reject! { |item| item['file_uri'] == file_uri }
25+
26+
@queue << {
27+
'file_uri' => file_uri,
28+
'doc_version' => doc_version,
29+
'document_type' => document_type,
30+
'workspace' => workspace,
31+
'connection_object' => connection_object
32+
}
33+
end
34+
35+
if @queue_thread.nil? || !@queue_thread.alive?
36+
@queue_thread = Thread.new do
37+
begin
38+
worker
39+
rescue => err # rubocop:disable Style/RescueStandardError
40+
PuppetLanguageServer.log_message(:error, "Error in ValidationQueue Thread: #{err}")
41+
raise
42+
end
43+
end
44+
end
45+
46+
nil
47+
end
48+
49+
# Synchronously validate a file
50+
def self.validate_sync(file_uri, doc_version, workspace, connection_object)
51+
document_type = connection_object.document_type(file_uri)
52+
content = documents.document(file_uri, doc_version)
53+
return nil if content.nil?
54+
result = validate(document_type, content, workspace)
55+
56+
# Send the response
57+
connection_object.reply_diagnostics(file_uri, result)
58+
end
59+
60+
# Helper method to the Document Store
61+
def self.documents
62+
PuppetLanguageServer::DocumentStore
63+
end
64+
65+
# Wait for the queue to become empty
66+
def self.drain_queue
67+
return if @queue_thread.nil? || !@queue_thread.alive?
68+
@queue_thread.join
69+
nil
70+
end
71+
72+
# Testing helper resets the queue and prepopulates it with
73+
# a known arbitrary configuration.
74+
# ONLY USE THIS FOR TESTING!
75+
def self.reset_queue(initial_state = [])
76+
@queue_mutex.synchronize do
77+
@queue = initial_state
78+
end
79+
end
80+
81+
# Validate a document
82+
def self.validate(document_type, content, workspace)
83+
# Perform validation
84+
case document_type
85+
when :manifest
86+
PuppetLanguageServer::DocumentValidator.validate(content, workspace)
87+
when :epp
88+
PuppetLanguageServer::DocumentValidator.validate_epp(content, workspace)
89+
else
90+
[]
91+
end
92+
end
93+
private_class_method :validate
94+
95+
# Thread worker which processes all jobs in the queue and validates each document
96+
# serially
97+
def self.worker
98+
work_item = nil
99+
loop do
100+
@queue_mutex.synchronize do
101+
return if @queue.empty?
102+
work_item = @queue.shift
103+
end
104+
return if work_item.nil?
105+
106+
file_uri = work_item['file_uri']
107+
doc_version = work_item['doc_version']
108+
connection_object = work_item['connection_object']
109+
document_type = work_item['document_type']
110+
workspace = work_item['workspace']
111+
112+
# Check if the document is the latest version
113+
content = documents.document(file_uri, doc_version)
114+
if content.nil?
115+
PuppetLanguageServer.log_message(:debug, "ValidationQueue Thread: Ignoring #{work_item['file_uri']} as it is not the latest version or has been removed")
116+
return
117+
end
118+
119+
# Perform validation
120+
result = validate(document_type, content, workspace)
121+
122+
# Check if the document is still latest version
123+
current_version = documents.document_version(file_uri)
124+
if current_version != doc_version
125+
PuppetLanguageServer.log_message(:debug, "ValidationQueue Thread: Ignoring #{work_item['file_uri']} as has changed version from #{doc_version} to #{current_version}")
126+
return
127+
end
128+
129+
# Send the response
130+
connection_object.reply_diagnostics(file_uri, result)
131+
end
132+
end
133+
private_class_method :worker
134+
end
135+
end

server/spec/languageserver/integration/puppet-languageserver/message_router_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
# Populate the document cache
2222
PuppetLanguageServer::DocumentStore.clear
23-
documents.each { |uri, content| PuppetLanguageServer::DocumentStore.set_document(uri, content) }
23+
documents.each { |uri, content| PuppetLanguageServer::DocumentStore.set_document(uri, content, 0) }
2424
end
2525

2626
context 'given a request that raises an error' do
@@ -95,7 +95,7 @@
9595

9696
# Populate the document cache
9797
PuppetLanguageServer::DocumentStore.clear
98-
documents.each { |uri, content| PuppetLanguageServer::DocumentStore.set_document(uri, content) }
98+
documents.each { |uri, content| PuppetLanguageServer::DocumentStore.set_document(uri, content, 0) }
9999
end
100100

101101
context 'given a request that raises an error' do

0 commit comments

Comments
 (0)