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.

Sign up for GitHub

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

Jump to bottom

(GH-209) Refactor the session state to be a class and pass that instead of global modules #210

Merged
merged 19 commits into from
Mar 31, 2020
Merged

(GH-209) Refactor the session state to be a class and pass that instead of global modules #210

merged 19 commits into from
Mar 31, 2020

Conversation

glennsarti
Copy link
Contributor

Fixes #209

The Puppet Editor Services - The PuppetHelper, queues and providers. have organically grown and are now a gnarled mess. Time to pay down some of the tech debt.

Refactor the from message-handler up in the stack.

  • Create session_state style class and attach it to the message_handler

  • Move sessions specific stuff into session_state

  • Refactor for new DocumentStore

    • Create a duck-type module for the new class
    • Make tests and rubocop pass
    • Finish refactoring EVERYTHING and get rid of the ducktype module

  • Refactor the PuppetHelper into the session_state where applicable

    • move loaders and xxxx_loaded? methods to session_state
    • Get rid of connection_id module variable
    • Get rid of cache module variable
    • ducktype?

  • Refactor object_cache

    • This is done as part of the PuppetHelper refactor

  • Refactor queues from modules to class

    • Single Instance queue
    • Validation
    • Sidecar
    • Migrate single instance tests from validation/sidecar to single_instance_queue

  • decouple object hierarchy in client_state
    Ideally we shouldn't need known object hierachies and should uses events/delegates
    e.g. don't do @connection_id = message_handler.protocol.connection.id

At this point the refactor is done but nothing has been improved

  • What improvements to make?
    • Make the documents a 1st class object, not a hash. Means we can cache AST and token lexing.
      Later PRs will add the caching etc. This PR only makes it possible to add those features.
  • Add End-to-End integration test (like the debug server) to smoke test the entire system. Seperate PR

@glennsarti glennsarti added this to the 1.0.0 milestone Dec 10, 2019
@glennsarti glennsarti force-pushed the refactor-client-state branch from 49546bd to a48035f Compare December 10, 2019 08:14
@jpogran jpogran self-requested a review January 16, 2020 15:11
@jpogran jpogran added the enhancement New feature or request label Jan 17, 2020
@glennsarti glennsarti force-pushed the refactor-client-state branch 4 times, most recently from 11ea8f6 to 5aa59de Compare February 3, 2020 12:40
@glennsarti glennsarti force-pushed the refactor-client-state branch 4 times, most recently from 0c8cd3f to f228b1a Compare March 31, 2020 03:05
@glennsarti
(puppetlabsGH-209) Create a session_state namespace
7a99d98 
This commit creates a session_state namespace.  This will hold the document
store, language client and the object cache. This just moves the files to
preserve git history.
@glennsarti
(puppetlabsGH-209) Create session_state and refactor DocumentStore
051a28b 
This commit:

* Creates a ClientSessionState object which holds all of the state information
  for a single client.
* Changes all of the object references to the new SessionState namespace.
* Adds a ducktype of the old PuppetLanguageServer::DocumentStore module and
  uses the ClientSessionState behind the scenes.  Later commits will remove
  this module.
@glennsarti
(maint) Return nil if the connection does not exist
1c41cf1 
Previously the TCP Server would return a hash instead of nil if the connection
ID did not exist. This commit forces the output to nil if the connectino does
not exist.
@glennsarti glennsarti force-pushed the refactor-client-state branch from f228b1a to 7e4bfa3 Compare March 31, 2020 03:21
glennsarti added 11 commits March 31, 2020 14:05
@glennsarti
(maint) Remove unused protocol object
1884907 
Previously the NodeGraph protocol object was changed to PuppetNodeGraph
however the original object was never deleted.  This commit deletes this unused
object.
@glennsarti
(puppetlabsGH-209) Refactor ValidationQueue into a class
f1c9fa4 
This commit:

* Creates a new base queue called SingleInstanceQueue.  This queue process jobs
  in one or more threads.  But only allows one job key to exist in the queue
  at once. When you add an additional job, it gets added to the end of the
  queue and any other jobs are removed.
* Creates a ValidationQueue which inherits from the SingleInstanceQueue but only
  has the extra things it needs to do validation.
* Creats a global module called GlobalQueues, which holds an instance of the
  validation queue.  Validation is a global queue (for any connection).
* Updates the spec tests for the new queue name
* Cleans up some of the logic in the queue.  It was pre-optimising but made it
  harder to follow through.
* The Validation Queue now uses the session_state for a given connection to make
  decisions instead of the global DocumentStore modulre.
@glennsarti
(puppetlabsGH-209) Refactor Crash Dump to use session state
33ed345 
This commit updates the Crash Dump to use the session_state instead of the
global DocumentStore module.
@glennsarti
(puppetlabsGH-209) Refactor Sidecar queue
d3a4242 
This commit:
* Uses the single_instance_queue for sidecar queue
* The Sidecar queue gets session_state via the connection_id property. Removing
  the calls to the global PuppetHelper module.
* Adds the connection_id getter and setter methods to the PuppetHelper to help
  transition the refactor.  These methods will be removed in later commits once
  the refactor is complete
* Because the Sidecar Queue now requires a connection_id and session_state it
  is no longer possible to preload the Puppet information without an actual (or
  mocked connection).  Therefore the spec_helper is updated to inject a JSON
  fixture file into the object cache instead of doing actual sidecar calls.
  This also speeds up tests run time as it no longer has to _actually_ do real
  queries, but just consume a test fixture.

 # Please enter the commit message for your changes. Lines starting
@glennsarti
(maint) Add acceptance test debugging
19ce735 
This commit adds commented out code which can be useful when debugging
acceptance tests.
@glennsarti
(puppetlabsGH-209) Refactor cache loaders and state detection
467f7b0 
This commit:

* Moves the cache loading helpers, and cache state query methods from the global
  PuppetHelper module into the Client Session State, where it makes more sense.
  The loading and querying depend on the state, specifically the object cache.
* Modifies the spec_helper to prepopulate an object cache from the test fixture.
  This again can speed up tests and means that some tests can be moved from
  integration into unit tests.
* Adds remove_origin!, origin_exist? and section_in_origin_exist? to the object
  cache as helper methods.
* Fixes a bug in the object cache which would raise erorrs for nils instead of
  handling them correctly
@glennsarti
(puppetlabsGH-209) Decouple session_state and connection id
3e43d18 
Previously when creating a ClientSessionState object it required a "real"
message handler.  Instead this commit updates the initializer so that you can
pass in exactly what the connection_id is, and fallback to message_handler way
if it's nil.  This makes it easier to test and mock.

This commit also removes any unneeded object creations for the session state in
the test suite.
@glennsarti
(puppetlabsGH-209) Refactor node_graph and puppet_resource helpers
83aa365 
This commit refactors the Puppet Helper to pass in the session state to the
get_node_graph and get_puppet_resource methods. Thereby making them pure
methods and easier to test.
@glennsarti
(puppetlabsGH-209) Refactor helper for type information
7f1934a 
Now that the basics are in place to pass session state around instead of using
global modules, it's time to start updating the object cache helpers in the
PuppetHelper module.  This commit only updates the type helpers.  Later commits
will update the other helpers.

This commit:
* Updates the two type helpers (get_type and type_names) to require the
  session_state to be passed in, instead of using the temporary connection_id
  method
* Updates the completion, definition and hover providers to call the updated
  type helpers with the session_state. This means that the session_state must
  be passed into these providers as well.
* Updates the message_handler to pass in the session state when calling the
  completion, definition and hover providers
* Updates the tests for the completion, definition and hover providers and
  message handler with the updated method signatures.
@glennsarti
(puppetlabsGH-209) Refactor helper for function information
59756e4 
Now that the basics are in place to pass session state around instead of using
global modules, it's time to start updating the object cache helpers in the
PuppetHelper module.  This commit only updates the function helpers.  Other
commits will update the other helpers.

This commit:
* Updates the two function helpers (function and function_names) to require the
  session_state to be passed in, instead of using the temporary connection_id
  method
* Updates the completion, definition, signature and hover providers to call the
  updated function helpers with the session_state. This means that the
  session_state must be passed into these providers as well.
* Updates the message_handler to pass in the session state when calling the
  completion, definition, signature and hover providers
* Updates the tests for the completion, definition, signature and hover
  providers and message handler with the updated method signatures.
@glennsarti
(puppetlabsGH-209) Refactor helper for class information
3ca893e 
Now that the basics are in place to pass session state around instead of using
global modules, it's time to start updating the object cache helpers in the
PuppetHelper module.  This commit only updates the class helpers.  Other
commits will update the other helpers.

This commit:
* Updates the two class helpers (get_class and class_names) to require the
  session_state to be passed in, instead of using the temporary connection_id
  method
* Updates the completion, definition and hover providers to call the
  updated class helpers with the session_state. This means that the
  session_state must be passed into these providers as well.
* Updates the message_handler to pass in the session state when calling the
  completion, definition and hover providers
* Updates the tests for the completion, definition and hover
  providers and message handler with the updated method signatures.
@glennsarti
(puppetlabsGH-209) Refactor helper for fact information
b677506 
Now that the basics are in place to pass session state around instead of using
global modules, it's time to start updating the object cache helpers in the
FacterHelper module.  This commit all the helpers.

This commit:
* Updates the all helpers to require the
  session_state to be passed in, instead of using the temporary connection_id
  method
* Updates the completion, definition and hover providers to call the
  updated class helpers with the session_state. This means that the
  session_state must be passed into these providers as well.
* Removes temporary methods that are not longer required.
@glennsarti
(puppetlabsGH-209) Refactor helper for datatype information
9de7b51 
Now that the basics are in place to pass session state around instead of using
global modules, it's time to start updating the object cache helpers in the
PuppetHelper module.  This commit only updates the class helpers.  Other
commits will update the other helpers.

This commit:
* Updates the one Datatype helpers (datatype) to require the
  session_state to be passed in, instead of using the temporary connection_id
  method
* Updates the hover providers to call the updated datatype helper with the
  session_state. This means that the session_state must be passed into this
  providers as well.
* Updates the message_handler to pass in the session state when calling the
  hover provider
* Updates the tests for the hover provider and message handler with the updated
  method signatures.
* Removes the now redundant inmemory_cache variable and .cache methods from the
  PupeptHelper as they are no longer required due to the refactor.
@glennsarti
(puppetlabsGH-209) Refactor references to DocumentStore to use sessio…
2dfb2e2 
…n_state

This commit removes the ducktyped DocumentStore module and changes all
references to use the Session State instead.
@glennsarti
(puppetlabsGH-209) Refactor single instance queue tests
1bd7e71 
Now that the sidecar and validation queue inherit from the SingleInstanceQueue
(SIQ), the common tests can be removed from those queues and applied only the
SIQ.  This commit only leaves the queue specific details in the queue specific
test files, for example the validation queue tests only assert on validation
based tasks, not that the queue is single instance.

This commit also updates the SingleInstanceQueueJob so that they key is passed
in at creation time, instead of requiring overriding an abstract method. The
key is immutable so there's no need to make it changeable.
@glennsarti
(puppetlabsGH-209) Refactor documents in the Document Store
7ebd435 
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
@glennsarti glennsarti force-pushed the refactor-client-state branch from 7e4bfa3 to 7ebd435 Compare March 31, 2020 06:06
@glennsarti glennsarti merged commit 8d8fc7f into puppetlabs:1.0 Mar 31, 2020
@glennsarti glennsarti deleted the refactor-client-state branch March 31, 2020 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpogran jpogran Awaiting requested review from jpogran

Assignees

@glennsarti glennsarti

Labels
enhancement New feature or request
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@glennsarti @jpogran