django-qsessions icon indicating copy to clipboard operation
django-qsessions copied to clipboard

Refactor: Clean-up file.stateBuilder

Open hbagdi opened this issue 3 years ago • 0 comments

stateBuilder is a function object that takes some input data (current state and target state, and kong version) and produces some output data.

  1. The distinction between inputs and outputs is not very clear in the implementation of stateBuilder. I would like to put on the table (for consideration) the possibility of refactoring stateBuilder into pure mathematical functions with no preserved state - similar to what happened to parser in KIC. I believe that this would help with testability and readability. Of course, let's be mindful of priorities in our projects - I only want these considerations to contribute to the vision of where we want to take the codebase to in small steps.
  2. When we think of stateBuilder as a mathematical function, like described above: A mathematical function changing its outputs based on version of some software dependency (here: gateway version) suffers from two problems: 2.1. keeps stateBuilder (a mathematical function processing X into Y) concerned with a version of "some external" component - which feels at odds with SRP 2.2. in tests, couples logic with software versions. I believe that the role of a unit test here would be to check that "state builder properly adds tags when asked to, and does not when not asked to". "Asking" the state builder to add tags or not depending on Gateway version is a separate concern imo, and should happen on a higher level (and would be tested on a higher level too, in an integration test).

To make the above considerations actionable, I believe that the following would help improve the code health:

  • kick kongVersion out of stateBuilder to a higher level, and replace it with knobs that control whether selectTags get applied to all creds, or all creds but mtls-auth, or to none.
  • refactor stateBuilder into pure stateless functions

Originally posted by @mflendrich in https://github.com/Kong/deck/pull/455#discussion_r665270706

hbagdi avatar Jul 07 '21 18:07 hbagdi