(GH-209) Refactor documents in the Document Store

Previously the document store just used a simple hash to store documents that
are being edited. However documents are important and should really by a first
class object type. This commit:
* Refactors the old `document` method to `document_content`.  Also all calls
  to the old method (including tests) were updated with the new method name.
* Refactors the document store so that documents come from the Document class.
* Adds stub classes for Epp, Manifest and Puppetfile document types. Later
  commits will actually use the different document classes for document specific
  tasks, e.g. caching AST, and tokens

lib/puppet-languageserver/crash_dump.rb

@@ -34,7 +34,7 @@ def self.write_crash_file(err, session_state, filename = nil, additional = {})
 
       # Append the documents in the cache
       session_state.documents.document_uris.each do |uri|
-        crashtext += "Document - #{uri}\n---\n#{session_state.documents.document(uri)}\n\n"
+        crashtext += "Document - #{uri}\n---\n#{session_state.documents.document_content(uri)}\n\n"
       end
       # Append additional objects from the crash
       additional.each do |k, v|

lib/puppet-languageserver/global_queues/validation_queue.rb

@@ -40,7 +40,7 @@ def execute_job(job_object)
         raise "Document store is not available for connection id #{job_object.connection_id}" if document_store.nil?
 
         # Check if the document is the latest version
-        content = document_store.document(job_object.file_uri, job_object.doc_version)
+        content = document_store.document_content(job_object.file_uri, job_object.doc_version)
         if content.nil?
           PuppetLanguageServer.log_message(:debug, "#{self.class.name}: Ignoring #{job_object.file_uri} as it is not the latest version or has been removed")
           return

lib/puppet-languageserver/message_handler.rb

@@ -97,7 +97,7 @@ def request_puppet_compilenodegraph(_, json_rpc_message)
     def request_puppet_fixdiagnosticerrors(_, json_rpc_message)
       formatted_request = LSP::PuppetFixDiagnosticErrorsRequest.new(json_rpc_message.params)
       file_uri = formatted_request.documentUri
-      content = documents.document(file_uri)
+      content = documents.document_content(file_uri)
 
       case documents.document_type(file_uri)
       when :manifest
@@ -126,7 +126,7 @@ def request_textdocument_completion(_, json_rpc_message)
       file_uri = json_rpc_message.params['textDocument']['uri']
       line_num = json_rpc_message.params['position']['line']
       char_num = json_rpc_message.params['position']['character']
-      content = documents.document(file_uri)
+      content = documents.document_content(file_uri)
       context = json_rpc_message.params['context'].nil? ? nil : LSP::CompletionContext.new(json_rpc_message.params['context'])
 
       case documents.document_type(file_uri)
@@ -152,7 +152,7 @@ def request_textdocument_hover(_, json_rpc_message)
       file_uri = json_rpc_message.params['textDocument']['uri']
       line_num = json_rpc_message.params['position']['line']
       char_num = json_rpc_message.params['position']['character']
-      content = documents.document(file_uri)
+      content = documents.document_content(file_uri)
       case documents.document_type(file_uri)
       when :manifest
         PuppetLanguageServer::Manifest::HoverProvider.resolve(session_state, content, line_num, char_num, :tasks_mode => documents.plan_file?(file_uri))
@@ -168,7 +168,7 @@ def request_textdocument_definition(_, json_rpc_message)
       file_uri = json_rpc_message.params['textDocument']['uri']
       line_num = json_rpc_message.params['position']['line']
       char_num = json_rpc_message.params['position']['character']
-      content = documents.document(file_uri)
+      content = documents.document_content(file_uri)
 
       case documents.document_type(file_uri)
       when :manifest
@@ -183,7 +183,7 @@ def request_textdocument_definition(_, json_rpc_message)
 
     def request_textdocument_documentsymbol(_, json_rpc_message)
       file_uri = json_rpc_message.params['textDocument']['uri']
-      content  = documents.document(file_uri)
+      content  = documents.document_content(file_uri)
 
       case documents.document_type(file_uri)
       when :manifest
@@ -201,7 +201,7 @@ def request_textdocument_ontypeformatting(_, json_rpc_message)
       file_uri = json_rpc_message.params['textDocument']['uri']
       line_num = json_rpc_message.params['position']['line']
       char_num = json_rpc_message.params['position']['character']
-      content  = documents.document(file_uri)
+      content  = documents.document_content(file_uri)
 
       case documents.document_type(file_uri)
       when :manifest
@@ -224,7 +224,7 @@ def request_textdocument_signaturehelp(_, json_rpc_message)
       file_uri = json_rpc_message.params['textDocument']['uri']
       line_num = json_rpc_message.params['position']['line']
       char_num = json_rpc_message.params['position']['character']
-      content  = documents.document(file_uri)
+      content  = documents.document_content(file_uri)
 
       case documents.document_type(file_uri)
       when :manifest

lib/puppet-languageserver/session_state/document_store.rb

@@ -2,6 +2,37 @@
 
 module PuppetLanguageServer
   module SessionState
+    # Represents a Document in the Document Store.
+    # Can be subclassed to add additional methods and helpers
+    class Document
+      attr_reader :uri
+      attr_reader :content
+      attr_reader :version
+
+      # @param uri String The content of the document
+      # @param content String The content of the document
+      # @param version Integer The version the document
+      def initialize(uri, content, version)
+        @uri = uri
+        @content = content
+        @version = version
+      end
+
+      # Update a document with new content and version
+      # @param content String The new content for the document
+      # @param version Integer The version of the new document
+      def update(content, version)
+        @content = content
+        @version = version
+      end
+    end
+
+    class EppDocument < Document; end
+
+    class ManifestDocument < Document; end
+
+    class PuppetfileDocument < Document; end
+
     class DocumentStore
       # @param options :workspace Path to the workspace
       def initialize(_options = {})
@@ -13,15 +44,17 @@ def initialize(_options = {})
 
       def set_document(uri, content, doc_version)
         @doc_mutex.synchronize do
-          @documents[uri] = {
-            :content => content,
-            :version => doc_version
-          }
+          if @documents[uri].nil?
+            @documents[uri] = create_document(uri, content, doc_version)
+          else
+            @documents[uri].update(content, doc_version)
+          end
         end
       end
 
       def remove_document(uri)
-        @doc_mutex.synchronize { @documents[uri] = nil }
+        @doc_mutex.synchronize { @documents.delete(uri) }
+        nil
       end
 
       def clear
@@ -31,16 +64,19 @@ def clear
       def document(uri, doc_version = nil)
         @doc_mutex.synchronize do
           return nil if @documents[uri].nil?
-          return nil unless doc_version.nil? || @documents[uri][:version] == doc_version
-          @documents[uri][:content].clone
+          return nil unless doc_version.nil? || @documents[uri].version == doc_version
+          @documents[uri]
         end
       end
 
+      def document_content(uri, doc_version = nil)
+        doc = document(uri, doc_version)
+        doc.nil? ? nil : doc.content.clone
+      end
+
       def document_version(uri)
-        @doc_mutex.synchronize do
-          return nil if @documents[uri].nil?
-          @documents[uri][:version]
-        end
+        doc = document(uri)
+        doc.nil? ? nil : doc.version
       end
 
       def document_uris
@@ -186,6 +222,20 @@ def windows?
         # library uses that to test what platform it's on.
         !!File::ALT_SEPARATOR # rubocop:disable Style/DoubleNegation
       end
+
+      # Creates a document object based on the Uri
+      def create_document(uri, content, doc_version)
+        case document_type(uri)
+        when :puppetfile
+          PuppetfileDocument.new(uri, content, doc_version)
+        when :manifest
+          ManifestDocument.new(uri, content, doc_version)
+        when :epp
+          EppDocument.new(uri, content, doc_version)
+        else
+          Document.new(uri, content, doc_version)
+        end
+      end
     end
   end
 end

spec/languageserver/unit/puppet-languageserver/message_handler_spec.rb

@@ -907,7 +907,7 @@
 
         it 'should add the document to the document store' do
           subject.notification_textdocument_didopen(connection_id, notification_message)
-          expect(subject.documents.document(file_uri)).to eq(file_content)
+          expect(subject.documents.document_content(file_uri)).to eq(file_content)
         end
 
         it 'should enqueue the file for validation' do
@@ -954,7 +954,7 @@
 
       it 'should remove the document from the document store' do
         subject.notification_textdocument_didclose(connection_id, notification_message)
-        expect(subject.documents.document(file_uri)).to be_nil
+        expect(subject.documents.document_content(file_uri)).to be_nil
       end
     end
 
@@ -979,7 +979,7 @@
 
         it 'should update the document in the document store' do
           subject.notification_textdocument_didchange(connection_id, notification_message)
-          expect(subject.documents.document(file_uri)).to eq(new_file_content)
+          expect(subject.documents.document_content(file_uri)).to eq(new_file_content)
         end
 
         it 'should enqueue the file for validation' do

spec/languageserver/unit/puppet-languageserver/session_state/document_store_spec.rb

@@ -3,6 +3,22 @@
 describe 'PuppetLanguageServer::SessionState::DocumentStore' do
   let(:subject) { PuppetLanguageServer::SessionState::DocumentStore.new }
 
+  describe '#set_document' do
+    let(:content) { 'content' }
+    let(:version) { 1 }
+    [
+      { :name => 'n EPP document', :uri => '/template.epp', :klass => PuppetLanguageServer::SessionState::EppDocument },
+      { :name => ' Puppet Manifest', :uri => '/manifest.pp', :klass => PuppetLanguageServer::SessionState::ManifestDocument },
+      { :name => ' Puppetfile', :uri => '/Puppetfile', :klass => PuppetLanguageServer::SessionState::PuppetfileDocument },
+      { :name => 'n unknown document', :uri => '/unknown.txt', :klass => PuppetLanguageServer::SessionState::Document },
+    ].each do |testcase|
+      it "creates a #{testcase[:klass]} object for a#{testcase[:name]}" do
+        subject.set_document(testcase[:uri], content, version)
+        expect(subject.document(testcase[:uri], version)).to be_a(testcase[:klass])
+      end
+    end
+  end
+
   describe '#plan_file?' do
     before(:each) do
       # Assume we are not in any module or control repo. Just a bare file