Skip to content
This repository was archived by the owner on Dec 20, 2023. It is now read-only.

Better data context #64

Merged
merged 2 commits into from
Nov 5, 2023
Merged

Better data context #64

merged 2 commits into from
Nov 5, 2023

Conversation

tiptenbrink
Copy link
Collaborator

This is a (relatively) minor refactor of the contexts dependency injection system I implemented in the recent large refactor. It's still the same idea, but the developer ergonomics are much improved and the weird application/auth duplication (which led to me calling the application implementation "frame" instead of complex) is removed. The library functions for this system now live in their own package, while the context.py in the auth and application packages only define the Contexts class and the stubs for all context functions.

Since this system wasn't explained in detail in #59, I'll try to give a short overview. I've also added better documentation in the docs.

We've had different versions of how database functions could be called:

  • Originally, they were simply top-level functions called directly from the application code. Because of import order reasons, this meant it was almost impossible to mock these functions, as they were often already defined as local variables in modules before they could be mocked.
  • Then, everything was moved to a big DbOperations class, which was instantiated at startup and called in every router function. I didn't like the look of this big class and the extra indentation that automatically brings. It did solve the mocking problem, however the mocker.patch solution had the function location as a string. Each time the function name changed, the refactoring often didn't change this. Plus, there was no real warning or error given if things didn't match.
  • Then, the DbOperations class was retired and everything moved back to its own top-level function. By always calling data functions from the actual data module (so not importing them as local variables and using data.<module>.<function>) mocker.patch now worked without that big class. This was Db refactor #56. The ergonomics when writing new routers was very good here, as you simply called the function you wanted, as long as you imported it in a specific way. However, it still suffered from the fact that mocker.patch requires a string.
  • Separation refactor #59 proved that mocker.patch was too fragile and that a changing implementation was too hard to deal with. So instead I moved to a dependency injection system, which was the current solution before this PR. It basically mixes the top-level and big class solution by creating a big class which contains only stubs (so the function name and parameters but not implementation, kinda like a header file) and then you define the implementation as a top-level function. By using decorators you collect all these implementations and then at application startup you replace the stubs with the implementations. However, the big problem was that you had to call the stubs on the big class object in the consuming application code. But if you then want to look up the implementation, you would only get to these stubs. There was no easy link to the actual main implementation as IDE's cannot follow the decorator magic. This was really bad ergonomics.

This PR solves this latter problem. There is still the stub class, but instead of calling . you can now call the top-level function that contains the main implementation directly. However, you provide the Context class as the first argument, which the decorator uses to lookup the desired implementation and replaces the function with that. The only downside is that each function now has this Context variable as its first arg, even though it appears unused in the actual implementation. This isn't a big deal though and might be fixable as IDE's improve.

So a short guide:

Each function that you want to be able to easily replace in test should live in a file in the context module of the package's respective data module.

  • Define a class (a registration type) that inherits from Context and contains a stub implementation. Do this only for a coherent group (like "login"). An instance of this is what will actually contain the function at runtime. This is the "dependency container"
  • Add an attribute in the Contexts class for this registration type. The Contexts class will live in the application's global state and will be used to obtain a reference to the container.
  • In the file where your function lives, define a ContextRegistry. This is a local container, only for the file, that is used only for registering the function in the correct global container at application startup.
  • You can use the register function to then register your function using a decorator. Provide the registry type (the actual class, not an instance) as an argument. This is necessary so we later know to what global container to add it.
  • At application startup, run the include_registry function for your global Contexts class on the local container ContextRegistry. All functions will be added to the correct global container contexts (the stub classes will get their implementation).
  • In consuming code, simply call the top-level function and provide it with the correct global container (the Context instance), which you can get from the Contexts class.
  • In tests, subclass the stub class and provide your own implementation. Then simply replace the attribute of the desired global container you want to replace on the application's Contexts class.

The only added work you are doing, considering usually a file and registration already exist, is adding the decorator and adding a stub. This is only a small amount of extra work. In the test you need to provide a mock implementation, but this was always necessary. If the implementation changes, the program will error if the stub does not match. Once the stub is fixed, the test implementation will now complain if it is no longer in sync. This way, it is easy to keep everything synced.

Unfortunately, as these functions were used quite a lot, especially in the auth package, this does lead to quite a few changed lines and files. This is really is a great DX improvement, though, so definitely worth it!

@tiptenbrink tiptenbrink merged commit 846c8de into main Nov 5, 2023
@tiptenbrink tiptenbrink deleted the better-data-context branch November 5, 2023 10:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant