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.

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

Implement Disable() on handler configurers. #289

Merged
merged 8 commits into from
Jul 16, 2024
Merged

Implement Disable() on handler configurers. #289

merged 8 commits into from
Jul 16, 2024

Conversation

jmalloc
Copy link
Member

@jmalloc jmalloc commented Jul 9, 2024

What change does this introduce?

This PR adds an IsDisabled() method to the Handler and RichHandler interfaces, the return value of which is driven by calls to the Disable() method on Dogma's handler configurer interfaces (AggregateConfigurer, etc).

Why make this change?

I wanted to prototype this before merging dogmatiq/dogma#162

Is there anything you are unsure about?

I haven't looked into adding support for Disable() to the static analysis system. I also haven't added support to the interopspec config API.

What issues does this relate to?

@jmalloc
Copy link
Member Author

jmalloc commented Jul 9, 2024

/cc @danilvpetrov FYI this will likely need some modifications to the static analysis code.

@jmalloc
Copy link
Member Author

jmalloc commented Jul 9, 2024

Thinking some more about the static analysis side of things, I'm not sure we can even report the disabled status in any meaningful way, because c.Disable() would always be called conditionally (otherwise the handler would always do nothing).

Edit: I guess we could handle the case where c.Disable() is called unconditionally, but otherwise just report the handler as enabled.

@jmalloc
Copy link
Member Author

jmalloc commented Jul 11, 2024

After discussion with @danilvpetrov, we decided to change the IsDisabled() method to instead return an enum of Enabled, Disabled and ConditionallyDisabled allowing us to represent all three possibilities from static analysis.

When the config is constructed from an in-memory application the return value will always be either Enabled or Disabled.

@dogmatiq dogmatiq deleted a comment from codecov bot Jul 16, 2024
@jmalloc
Copy link
Member Author

jmalloc commented Jul 16, 2024

I've updated this PR to exclude any handlers that are disabled and otherwise unconfigured (i.e. no identity or routes) from application configurations.

@jmalloc jmalloc marked this pull request as ready for review July 16, 2024 02:12
@jmalloc jmalloc requested review from koden-km and danilvpetrov July 16, 2024 02:24
@jmalloc jmalloc merged commit 20d2477 into main Jul 16, 2024
3 checks passed
@jmalloc jmalloc deleted the handler-disable branch July 16, 2024 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants