diff --git a/app.go b/app.go index a5e269908..ac2a6c335 100644 --- a/app.go +++ b/app.go @@ -436,6 +436,13 @@ func New(opts ...Option) *App { } app.modules = append(app.modules, app.root) + for _, opt := range opts { + if _, ok := opt.(privateOption); ok { + app.root.private = true + break + } + } + for _, opt := range opts { opt.apply(app.root) } diff --git a/app_test.go b/app_test.go index 9f705834e..45d69fed0 100644 --- a/app_test.go +++ b/app_test.go @@ -383,136 +383,6 @@ func TestNewApp(t *testing.T) { }) } -func TestPrivateProvide(t *testing.T) { - t.Parallel() - - t.Run("CanUsePrivateAndNonPrivateFromOuterModule", func(t *testing.T) { - t.Parallel() - - app := fxtest.New(t, - Module("SubModule", Invoke(func(a int, b string) {})), - Provide(func() int { return 0 }, Private), - Provide(func() string { return "" }), - ) - app.RequireStart().RequireStop() - }) - - t.Run("CantUsePrivateFromSubModule", func(t *testing.T) { - t.Parallel() - - app := NewForTest(t, - Module("SubModule", Provide(func() int { return 0 }, Private)), - Invoke(func(a int) {}), - ) - err := app.Err() - require.Error(t, err) - assert.Contains(t, err.Error(), "missing dependencies for function") - assert.Contains(t, err.Error(), "missing type: int") - }) - - t.Run("DifferentModulesCanProvideSamePrivateType", func(t *testing.T) { - t.Parallel() - - app := fxtest.New(t, - Module("SubModuleA", - Provide(func() int { return 1 }, Private), - Invoke(func(s int) { - assert.Equal(t, 1, s) - }), - ), - Module("SubModuleB", - Provide(func() int { return 2 }, Private), - Invoke(func(s int) { - assert.Equal(t, 2, s) - }), - ), - Provide(func() int { return 3 }), - Invoke(func(s int) { - assert.Equal(t, 3, s) - }), - ) - app.RequireStart().RequireStop() - }) -} - -func TestPrivateProvideWithDecorators(t *testing.T) { - t.Parallel() - - t.Run("DecoratedPublicOrPrivateTypeInSubModule", func(t *testing.T) { - t.Parallel() - - runApp := func(private bool) { - provideOpts := []interface{}{func() int { return 0 }} - if private { - provideOpts = append(provideOpts, Private) - } - app := NewForTest(t, - Module("SubModule", - Provide(provideOpts...), - Decorate(func(a int) int { return a + 2 }), - Invoke(func(a int) { assert.Equal(t, 2, a) }), - ), - Invoke(func(a int) { assert.Equal(t, 0, a) }), - ) - err := app.Err() - if private { - require.Error(t, err) - assert.Contains(t, err.Error(), "missing dependencies for function") - assert.Contains(t, err.Error(), "missing type: int") - } else { - require.NoError(t, err) - } - } - - t.Run("Public", func(t *testing.T) { runApp(false) }) - t.Run("Private", func(t *testing.T) { runApp(true) }) - }) - - t.Run("DecoratedPublicOrPrivateTypeInOuterModule", func(t *testing.T) { - t.Parallel() - - runApp := func(private bool) { - provideOpts := []interface{}{func() int { return 0 }} - if private { - provideOpts = append(provideOpts, Private) - } - app := fxtest.New(t, - Provide(provideOpts...), - Decorate(func(a int) int { return a - 5 }), - Invoke(func(a int) { - assert.Equal(t, -5, a) - }), - Module("Child", - Decorate(func(a int) int { return a + 10 }), - Invoke(func(a int) { - assert.Equal(t, 5, a) - }), - ), - ) - app.RequireStart().RequireStop() - } - - t.Run("Public", func(t *testing.T) { runApp(false) }) - t.Run("Private", func(t *testing.T) { runApp(true) }) - }) - - t.Run("CannotDecoratePrivateChildType", func(t *testing.T) { - t.Parallel() - - app := NewForTest(t, - Module("Child", - Provide(func() int { return 0 }, Private), - ), - Decorate(func(a int) int { return a + 5 }), - Invoke(func(a int) {}), - ) - err := app.Err() - require.Error(t, err) - assert.Contains(t, err.Error(), "missing dependencies for function") - assert.Contains(t, err.Error(), "missing type: int") - }) -} - func TestWithLoggerErrorUseDefault(t *testing.T) { // This test cannot be run in paralllel with the others because // it hijacks stderr. diff --git a/docs/modules.md b/docs/modules.md index 41f28e045..478bf7391 100644 --- a/docs/modules.md +++ b/docs/modules.md @@ -80,6 +80,9 @@ To write an Fx module: No modules that contain "server" will be able to use the resulting `Config` type because it can only be seen by the "server" module. + You can also pass `fx.Private` to a module to mark all + provided and supplied contents of that module as private. + That's all there's to writing modules. The rest of this section covers standards and conventions we've established for writing Fx modules at Uber. diff --git a/fxevent/console.go b/fxevent/console.go index 320fdfd4d..0fa2e7ba3 100644 --- a/fxevent/console.go +++ b/fxevent/console.go @@ -60,12 +60,16 @@ func (l *ConsoleLogger) LogEvent(event Event) { l.logf("HOOK OnStop\t\t%s called by %s ran successfully in %s", e.FunctionName, e.CallerName, e.Runtime) } case *Supplied: + var privateStr string + if e.Private { + privateStr = " (PRIVATE)" + } if e.Err != nil { l.logf("ERROR\tFailed to supply %v: %+v", e.TypeName, e.Err) } else if e.ModuleName != "" { - l.logf("SUPPLY\t%v from module %q", e.TypeName, e.ModuleName) + l.logf("SUPPLY%v\t%v from module %q", privateStr, e.TypeName, e.ModuleName) } else { - l.logf("SUPPLY\t%v", e.TypeName) + l.logf("SUPPLY%v\t%v", privateStr, e.TypeName) } case *Provided: var privateStr string diff --git a/fxevent/console_test.go b/fxevent/console_test.go index 6d54ff234..d46fd42a4 100644 --- a/fxevent/console_test.go +++ b/fxevent/console_test.go @@ -137,14 +137,32 @@ func TestConsoleLogger(t *testing.T) { }, { name: "Supplied", - give: &Supplied{TypeName: "*bytes.Buffer"}, + give: &Supplied{TypeName: "*bytes.Buffer", Private: false}, want: "[Fx] SUPPLY *bytes.Buffer\n", }, { name: "Supplied with module", - give: &Supplied{TypeName: "*bytes.Buffer", ModuleName: "myModule"}, + give: &Supplied{ + TypeName: "*bytes.Buffer", + ModuleName: "myModule", + Private: false, + }, want: "[Fx] SUPPLY *bytes.Buffer from module \"myModule\"\n", }, + { + name: "Supplied privately", + give: &Supplied{TypeName: "*bytes.Buffer", Private: true}, + want: "[Fx] SUPPLY (PRIVATE) *bytes.Buffer\n", + }, + { + name: "Supplied with module privately", + give: &Supplied{ + TypeName: "*bytes.Buffer", + ModuleName: "myModule", + Private: true, + }, + want: "[Fx] SUPPLY (PRIVATE) *bytes.Buffer from module \"myModule\"\n", + }, { name: "SuppliedError", give: &Supplied{TypeName: "*bytes.Buffer", Err: errors.New("great sadness")}, diff --git a/fxevent/event.go b/fxevent/event.go index c367f730c..a14590651 100644 --- a/fxevent/event.go +++ b/fxevent/event.go @@ -114,6 +114,9 @@ type Supplied struct { // Err is non-nil if we failed to supply the value. Err error + + // Private denotes whether the supplied value is [Private] or not + Private bool } // Provided is emitted when a constructor is provided to Fx. diff --git a/fxevent/zap.go b/fxevent/zap.go index b558b2ac8..2b2ac6864 100644 --- a/fxevent/zap.go +++ b/fxevent/zap.go @@ -110,6 +110,7 @@ func (l *ZapLogger) LogEvent(event Event) { l.logEvent("supplied", zap.String("type", e.TypeName), moduleField(e.ModuleName), + maybeBool("private", e.Private), ) } case *Provided: diff --git a/fxevent/zap_test.go b/fxevent/zap_test.go index c3d378080..38505b3ab 100644 --- a/fxevent/zap_test.go +++ b/fxevent/zap_test.go @@ -129,13 +129,28 @@ func TestZapLogger(t *testing.T) { }, }, { - name: "Supplied", - give: &Supplied{TypeName: "*bytes.Buffer"}, + name: "Supplied", + give: &Supplied{ + TypeName: "*bytes.Buffer", + Private: false, + }, wantMessage: "supplied", wantFields: map[string]interface{}{ "type": "*bytes.Buffer", }, }, + { + name: "PrivateSupply", + give: &Supplied{ + TypeName: "*bytes.Buffer", + Private: true, + }, + wantMessage: "supplied", + wantFields: map[string]interface{}{ + "type": "*bytes.Buffer", + "private": true, + }, + }, { name: "Supplied/Error", give: &Supplied{TypeName: "*bytes.Buffer", Err: someError}, diff --git a/module.go b/module.go index 7a28ca8d3..70aad3d0a 100644 --- a/module.go +++ b/module.go @@ -69,10 +69,22 @@ func (o moduleOption) apply(mod *module) { // 2. Apply child Options on the new module. // 3. Append it to the parent module. newModule := &module{ - name: o.name, - parent: mod, - app: mod.app, + name: o.name, + parent: mod, + app: mod.app, + private: mod.private, } + + // Look for private option first, if not set through parent + if !newModule.private { + for _, opt := range o.options { + if _, ok := opt.(privateOption); ok { + newModule.private = true + break + } + } + } + for _, opt := range o.options { opt.apply(newModule) } @@ -83,6 +95,7 @@ type module struct { parent *module name string scope scope + private bool provides []provide invokes []invoke decorators []decorator @@ -157,6 +170,7 @@ func (m *module) provide(p provide) { TypeName: p.SupplyType.String(), ModuleName: m.name, Err: m.app.err, + Private: p.Private, } default: diff --git a/private.go b/private.go new file mode 100644 index 000000000..9043d4974 --- /dev/null +++ b/private.go @@ -0,0 +1,58 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package fx + +type privateOption struct{} + +// Private is an [Option] that will mark the scope of a module as private. +// When a module is private, all of its provided and supplied contents +// (including those of inner modules that the private modules contains) +// cannot be accessed by outer modules that contain the private module. +// +// fx.New( +// fx.Module("SubModule", +// fx.Private, +// fx.Supply(5), +// fx.Provide(func() string { return "b"}), +// fx.Invoke(a int, b str) {}, // Runs w/ a = 5, b = "b" +// ), +// fx.Invoke(func(a int) {}), // This will fail +// fx.Invoke(func(b str) {}), // This will also fail +// ) +// +// Private can also be used directly as an argument in a call to [Provide] to +// mark only those provided functions as private, rather than a whole module. +// +// fx.New( +// fx.Module("SubModule", +// fx.Provide(func() int { return 0 }, fx.Private), +// fx.Provide(func() string { return "b" }), +// ), +// fx.Invoke(func(a int) {}), // This will fail +// fx.Invoke(func(b str) {}), // This will not +// ) +var Private = privateOption{} + +func (o privateOption) String() string { + return "fx.Private" +} + +func (o privateOption) apply(mod *module) {} diff --git a/private_test.go b/private_test.go new file mode 100644 index 000000000..427f198fb --- /dev/null +++ b/private_test.go @@ -0,0 +1,220 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package fx_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + . "go.uber.org/fx" + "go.uber.org/fx/fxtest" +) + +func TestPrivateModule(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + name string + getApp func(bool) *App + wantErr string + }{ + { + name: "Private Supply", + getApp: func(private bool) *App { + opts := []Option{Supply(5)} + if private { + opts = append(opts, Private) + } + return New( + Module("SubModule", opts...), + Invoke(func(a int) {}), + ) + }, + wantErr: "missing type: int", + }, + { + name: "Deeply Nested Modules", + getApp: func(private bool) *App { + opts := []Option{ + Module("ModuleB", + Module("ModuleC", + Provide(func() string { return "s" }), + ), + ), + } + if private { + opts = append(opts, Private) + } + return New( + Module("ModuleA", opts...), + Invoke(func(a string) {}), + ) + }, + wantErr: "missing type: string", + }, + } { + t.Run(tt.name, func(t *testing.T) { + t.Run("Public", func(t *testing.T) { + app := tt.getApp(false) + require.NoError(t, app.Err()) + }) + + t.Run("Private", func(t *testing.T) { + app := tt.getApp(true) + err := app.Err() + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + }) + }) + } +} + +func TestPrivateProvide(t *testing.T) { + t.Parallel() + + t.Run("CanUsePrivateAndNonPrivateFromOuterModule", func(t *testing.T) { + t.Parallel() + + app := fxtest.New(t, + Module("SubModule", Invoke(func(a int, b string) {})), + Provide(func() int { return 0 }, Private), + Provide(func() string { return "" }), + ) + app.RequireStart().RequireStop() + }) + + t.Run("CantUsePrivateFromSubModule", func(t *testing.T) { + t.Parallel() + + app := NewForTest(t, + Module("SubModule", Provide(func() int { return 0 }, Private)), + Invoke(func(a int) {}), + ) + err := app.Err() + require.Error(t, err) + assert.Contains(t, err.Error(), "missing dependencies for function") + assert.Contains(t, err.Error(), "missing type: int") + }) + + t.Run("DifferentModulesCanProvideSamePrivateType", func(t *testing.T) { + t.Parallel() + + app := fxtest.New(t, + Module("SubModuleA", + Provide(func() int { return 1 }, Private), + Invoke(func(s int) { + assert.Equal(t, 1, s) + }), + ), + Module("SubModuleB", + Provide(func() int { return 2 }, Private), + Invoke(func(s int) { + assert.Equal(t, 2, s) + }), + ), + Provide(func() int { return 3 }), + Invoke(func(s int) { + assert.Equal(t, 3, s) + }), + ) + app.RequireStart().RequireStop() + }) +} + +func TestPrivateProvideWithDecorators(t *testing.T) { + t.Parallel() + + t.Run("DecoratedPublicOrPrivateTypeInSubModule", func(t *testing.T) { + t.Parallel() + + runApp := func(private bool) { + provideOpts := []interface{}{func() int { return 0 }} + if private { + provideOpts = append(provideOpts, Private) + } + app := NewForTest(t, + Module("SubModule", + Provide(provideOpts...), + Decorate(func(a int) int { return a + 2 }), + Invoke(func(a int) { assert.Equal(t, 2, a) }), + ), + Invoke(func(a int) { assert.Equal(t, 0, a) }), + ) + err := app.Err() + if private { + require.Error(t, err) + assert.Contains(t, err.Error(), "missing dependencies for function") + assert.Contains(t, err.Error(), "missing type: int") + } else { + require.NoError(t, err) + } + } + + t.Run("Public", func(t *testing.T) { runApp(false) }) + t.Run("Private", func(t *testing.T) { runApp(true) }) + }) + + t.Run("DecoratedPublicOrPrivateTypeInOuterModule", func(t *testing.T) { + t.Parallel() + + runApp := func(private bool) { + provideOpts := []interface{}{func() int { return 0 }} + if private { + provideOpts = append(provideOpts, Private) + } + app := fxtest.New(t, + Provide(provideOpts...), + Decorate(func(a int) int { return a - 5 }), + Invoke(func(a int) { + assert.Equal(t, -5, a) + }), + Module("Child", + Decorate(func(a int) int { return a + 10 }), + Invoke(func(a int) { + assert.Equal(t, 5, a) + }), + ), + ) + app.RequireStart().RequireStop() + } + + t.Run("Public", func(t *testing.T) { runApp(false) }) + t.Run("Private", func(t *testing.T) { runApp(true) }) + }) + + t.Run("CannotDecoratePrivateChildType", func(t *testing.T) { + t.Parallel() + + app := NewForTest(t, + Module("Child", + Provide(func() int { return 0 }, Private), + ), + Decorate(func(a int) int { return a + 5 }), + Invoke(func(a int) {}), + ) + err := app.Err() + require.Error(t, err) + assert.Contains(t, err.Error(), "missing dependencies for function") + assert.Contains(t, err.Error(), "missing type: int") + }) +} diff --git a/provide.go b/provide.go index eb61ce999..81abc9607 100644 --- a/provide.go +++ b/provide.go @@ -74,7 +74,7 @@ type provideOption struct { } func (o provideOption) apply(mod *module) { - var private bool + private := mod.private targets := make([]interface{}, 0, len(o.Targets)) for _, target := range o.Targets { @@ -94,23 +94,6 @@ func (o provideOption) apply(mod *module) { } } -type privateOption struct{} - -// Private is an option that can be passed as an argument to [Provide] to -// restrict access to the constructors being provided. Specifically, -// corresponding constructors can only be used within the current module -// or modules the current module contains. Other modules that contain this -// module won't be able to use the constructor. -// -// For example, the following would fail because the app doesn't have access -// to the inner module's constructor. -// -// fx.New( -// fx.Module("SubModule", fx.Provide(func() int { return 0 }, fx.Private)), -// fx.Invoke(func(a int) {}), -// ) -var Private = privateOption{} - func (o provideOption) String() string { items := make([]string, len(o.Targets)) for i, c := range o.Targets { diff --git a/supply.go b/supply.go index f3997289e..c8408fe78 100644 --- a/supply.go +++ b/supply.go @@ -117,6 +117,7 @@ func (o supplyOption) apply(m *module) { Stack: o.Stack, IsSupply: true, SupplyType: o.Types[i], + Private: m.private, }) } }