hashicorp / hashicorp/consul

Replace function registry pattern

Open
#11,740 0 comments 0 reactions 0 assignees View on GitHub
theme/internal-cleanup
Dominant language
Go
Stars
30.1k
Forks
4.6k
Avg merge
1d 18h
Merged PRs (30d)
39

Description

There are a number of packages (see below for the list) where we use a registry to store functions in a map. In 05d5ec480497a0b0952eba2de5305e611cfb6d73 we replaced one of those instances with a different pattern. The new pattern was simply a function that accepted a list of other functions.

The new pattern of simply calling functions directly has a few advantages over the "function registry" pattern:
* it is much easier to trace the code. The function registry pattern requires many hops (init -> register -> map definition -> map usage), with a direct function call it is simply one hop (function -> caller).
* it is easier to write a reasonable test when the code being tested accepts its dependencies as inputs, instead of them being declared statically.
* in some cases this pattern forces us to keep all the functions in the same package, or even as methods on the same struct (because the registry is of unbound methods instead of functions). This leads to very large packages and sub-optimal code layout which cause significant developer productivity issues.

This issue proposes we addresses these problems by replacing each of these instances of the "function registry" pattern with direct function calls, similar to 05d5ec480497a0b0952eba2de5305e611cfb6d73.

We can see this pattern in the following places:

* CLI commands - `command/commands_oss.go`
* HTTP handlers - `agent/http_register.go`
* FSM operations - `agent/consul/fsm/commands_oss.go`
* FSM snapshot persist functions - `agent/consul/fsm/snapshot_oss.go`
* FSM snapshot restore functions - `agent/consul/fsm/snapshot_oss.go`
* ACL auth methods - `agent/consul/authmethod`
* RPC handlers - `agent/consul/server_register.go`
* Watch functions - `api/watch/funcs.go`

Contributor guide

Open the contributing guide

Research direction

Start by reading commit 05d5ec480497a0b0952eba2de5305e611cfb6d73, then compare the registry patterns in command/commands_oss.go, agent/http_register.go, agent/consul/fsm/commands_oss.go, agent/consul/fsm/snapshot_oss.go, agent/consul/authmethod, agent/consul/server_register.go, and api/watch/funcs.go. Done means each listed registry instance has been replaced with direct function calls and the affected behavior remains covered by the project’s tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend, cli, distributed-systems
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.