The mistake stems from the fact that the route function routes requests to the exact version of the function it is given, not the version after all endpoints are applied. This means that the order of decorators matters when route is used; this is a fairly subtle mistake, so it is easy to make and hard to notice just by reading over the code. There needs to be some way of preventing this bug from being introduced at all.
I see three initially plausible solutions.
Save a copy of each function that is returned from the decorator, and add tests that iterate over the list testing if each corresponding endpoint actually implements the decorators behavior. Setting aside the issue of how to find the corresponding endpoints, this is not viable because in general the decorators would have no way of knowing what parameters to use to make a successful call to the endpoint. Moreover, this approach requires that the code correctly accumulate the list of functions it is applied to in order for the tests to be valid. As a first approximation, I think that any addition to code for the sake of testing is an anti-pattern because practically by definition it leads to spaghetti code with test logic being distributed between the test file and the file being tested. Additionally, the code being added will also need to be tested which just makes writing tests take that much more work.
Modify each of my decorators so that they can detect if they are being applied to a function that has already been wrapped by bp.route and fail if that is the case. This is not possible because bp.routedoes not "modify" the function it is applied to, so its application cannot be detected. We could make a LifeLogServer.myroute decorator to watermark functions that have been routed, thus solving the detectability problem. We could even make a new decorator that would be applied to all of my existing decorators that would make them fail when the watermark is detected. There is a lot of appeal to this approach, but I hesitate because it once again requires modifying code for the sake of testing, even if this modification is much less out of place than collecting a list of functions that is handed directly to the test modules.
Perhaps the most straightforward approach is to add some sort of register_authenticated_endpoint test function for each class of decorator, then for each application of the decorator add a test that calls that test function. There are a few different appeals to this approach; first, it does not require any changes to the code thus keeping it nice and simple, and second, this tests the actual behavior of the endpoints (which is what we ultimately care about) rather than monitoring the internal state as an indicator of how the endpoints would behave. The primary drawback I see to this approach is that it is verbose from a testing perspective, it requires an additional test for each application of each decorator, rather than a handful of tests that test the decorator itself. However, that's not entirely unreasonable; testing the decorators in isolation would be unit testing, but this issue is one of integration so it makes sense to have many integrations tests to solve it.
If it isn't already clear, I'm inclined towards using solution number three.
As intended, this code requires that users authenticate before using the
/getendpoint:Somewhat unexpectedly, in the following example the
getfunction does require authentication, but the/getendpoint does not.The mistake stems from the fact that the
routefunction routes requests to the exact version of the function it is given, not the version after all endpoints are applied. This means that the order of decorators matters whenrouteis used; this is a fairly subtle mistake, so it is easy to make and hard to notice just by reading over the code. There needs to be some way of preventing this bug from being introduced at all.I see three initially plausible solutions.
Save a copy of each function that is returned from the decorator, and add tests that iterate over the list testing if each corresponding endpoint actually implements the decorators behavior. Setting aside the issue of how to find the corresponding endpoints, this is not viable because in general the decorators would have no way of knowing what parameters to use to make a successful call to the endpoint. Moreover, this approach requires that the code correctly accumulate the list of functions it is applied to in order for the tests to be valid. As a first approximation, I think that any addition to code for the sake of testing is an anti-pattern because practically by definition it leads to spaghetti code with test logic being distributed between the test file and the file being tested. Additionally, the code being added will also need to be tested which just makes writing tests take that much more work.
Modify each of my decorators so that they can detect if they are being applied to a function that has already been wrapped by
bp.routeand fail if that is the case. This is not possible becausebp.routedoes not "modify" the function it is applied to, so its application cannot be detected. We could make aLifeLogServer.myroutedecorator to watermark functions that have been routed, thus solving the detectability problem. We could even make a new decorator that would be applied to all of my existing decorators that would make them fail when the watermark is detected. There is a lot of appeal to this approach, but I hesitate because it once again requires modifying code for the sake of testing, even if this modification is much less out of place than collecting a list of functions that is handed directly to the test modules.Perhaps the most straightforward approach is to add some sort of
register_authenticated_endpointtest function for each class of decorator, then for each application of the decorator add a test that calls that test function. There are a few different appeals to this approach; first, it does not require any changes to the code thus keeping it nice and simple, and second, this tests the actual behavior of the endpoints (which is what we ultimately care about) rather than monitoring the internal state as an indicator of how the endpoints would behave. The primary drawback I see to this approach is that it is verbose from a testing perspective, it requires an additional test for each application of each decorator, rather than a handful of tests that test the decorator itself. However, that's not entirely unreasonable; testing the decorators in isolation would be unit testing, but this issue is one of integration so it makes sense to have many integrations tests to solve it.If it isn't already clear, I'm inclined towards using solution number three.