Skip to content

Commit

Permalink
refac: DRY up New, reuse *All methods, delete modules slice
Browse files Browse the repository at this point in the history
The pattern we appear to have for Fx operations (Provide, Invoke, etc.)
take the form:

    func (*module) thing(t thing)

    func (m *module) thingAll() {
        // perform module-local version of operation
        for _, t := range m.things {
            m.thing(t)
        }

        // recurse into children
        for _, m := range m.modules {
            m.${operation}All()
        }
    }

This means that the following two are equivalent:

    // 1
    for _, m := range app.modules {
        m.thingAll()
    }

    // 2
    app.root.thingAll()

Except (2) is DRYer.

This cleans up New by relying on root-level `*All` methods
instead of manually iterating over modules.

Making this change also highlighted that 'app.modules'
only ever has one entry: the root module.
So we can delete that field from App as well.

Finally, this also renames:

    constructAllCustomLoggers -> installAllEventLoggers
    constructCustomLogger     -> installEventLogger
    executeInvokes            -> invokeAll
    executeInvoke             -> invoke
  • Loading branch information
abhinav committed Jan 9, 2025
1 parent 7eebf3c commit c0aea1b
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 25 deletions.
22 changes: 6 additions & 16 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,6 @@ type App struct {

container *dig.Container
root *module
modules []*module

// Timeouts used
startTimeout time.Duration
Expand Down Expand Up @@ -446,7 +445,6 @@ func New(opts ...Option) *App {
log: logger,
trace: []string{fxreflect.CallerStack(1, 2)[0].String()},
}
app.modules = append(app.modules, app.root)

for _, opt := range opts {
opt.apply(app.root)
Expand Down Expand Up @@ -475,10 +473,7 @@ func New(opts ...Option) *App {
}

app.container = dig.New(containerOptions...)

for _, m := range app.modules {
m.build(app, app.container)
}
app.root.build(app, app.container)

// Provide Fx types first to increase the chance a custom logger
// can be successfully built in the face of unrelated DI failure.
Expand All @@ -490,31 +485,26 @@ func New(opts ...Option) *App {
})
app.root.provide(provide{Target: app.shutdowner, Stack: frames})
app.root.provide(provide{Target: app.dotGraph, Stack: frames})
app.root.provideAll()

for _, m := range app.modules {
m.provideAll()
}

// Run decorators before executing any Invokes -- including the one
// inside constructCustomLogger.
// Run decorators before executing any Invokes
// (including the ones inside installAllEventLoggers).
app.err = multierr.Append(app.err, app.root.decorateAll())

// If you are thinking about returning here after provides: do not (just yet)!
// If a custom logger was being used, we're still buffering messages.
// We'll want to flush them to the logger.

// custom app logger will be initialized by the root module.
for _, m := range app.modules {
m.constructAllCustomLoggers()
}
app.root.installAllEventLoggers()

// This error might have come from the provide loop above. We've
// already flushed to the custom logger, so we can return.
if app.err != nil {
return app
}

if err := app.root.executeInvokes(); err != nil {
if err := app.root.invokeAll(); err != nil {
app.err = err

if dig.CanVisualizeError(err) {
Expand Down
17 changes: 8 additions & 9 deletions module.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,11 @@ func (m *module) supply(p provide) {
}

// Constructs custom loggers for all modules in the tree
func (m *module) constructAllCustomLoggers() {
func (m *module) installAllEventLoggers() {
if m.logConstructor != nil {
if buffer, ok := m.log.(*logBuffer); ok {
// default to parent's logger if custom logger constructor fails
if err := m.constructCustomLogger(buffer); err != nil {
if err := m.installEventLogger(buffer); err != nil {
m.app.err = multierr.Append(m.app.err, err)
m.log = m.fallbackLogger
buffer.Connect(m.log)
Expand All @@ -269,12 +269,11 @@ func (m *module) constructAllCustomLoggers() {
}

for _, mod := range m.modules {
mod.constructAllCustomLoggers()
mod.installAllEventLoggers()
}
}

// Mirroring the behavior of app.constructCustomLogger
func (m *module) constructCustomLogger(buffer *logBuffer) (err error) {
func (m *module) installEventLogger(buffer *logBuffer) (err error) {
p := m.logConstructor
fname := fxreflect.FuncName(p.Target)
defer func() {
Expand All @@ -297,23 +296,23 @@ func (m *module) constructCustomLogger(buffer *logBuffer) (err error) {
})
}

func (m *module) executeInvokes() error {
func (m *module) invokeAll() error {
for _, m := range m.modules {
if err := m.executeInvokes(); err != nil {
if err := m.invokeAll(); err != nil {
return err
}
}

for _, invoke := range m.invokes {
if err := m.executeInvoke(invoke); err != nil {
if err := m.invoke(invoke); err != nil {
return err
}
}

return nil
}

func (m *module) executeInvoke(i invoke) (err error) {
func (m *module) invoke(i invoke) (err error) {
fnName := fxreflect.FuncName(i.Target)
m.log.LogEvent(&fxevent.Invoking{
FunctionName: fnName,
Expand Down

0 comments on commit c0aea1b

Please sign in to comment.