fix: address Codex review findings on PR #28
- WithOptions copies the Options slice (constructor isolation regression) - WithService auto-discovers service name from package path via reflect - WithService auto-registers HandleIPCEvents if present (v0.3.3 parity) - Add test for failing option short-circuit in New() Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
parent
c45b22849f
commit
b5dcdbb216
2 changed files with 55 additions and 3 deletions
43
contract.go
43
contract.go
|
|
@ -6,6 +6,7 @@ package core
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"reflect"
|
||||||
)
|
)
|
||||||
|
|
||||||
// Message is the type for IPC broadcasts (fire-and-forget).
|
// Message is the type for IPC broadcasts (fire-and-forget).
|
||||||
|
|
@ -120,8 +121,10 @@ func New(opts ...CoreOption) Result {
|
||||||
// core.WithOptions(core.Options{{Key: "name", Value: "myapp"}})
|
// core.WithOptions(core.Options{{Key: "name", Value: "myapp"}})
|
||||||
func WithOptions(opts Options) CoreOption {
|
func WithOptions(opts Options) CoreOption {
|
||||||
return func(c *Core) Result {
|
return func(c *Core) Result {
|
||||||
c.options = &opts
|
cp := make(Options, len(opts))
|
||||||
if name := opts.String("name"); name != "" {
|
copy(cp, opts)
|
||||||
|
c.options = &cp
|
||||||
|
if name := cp.String("name"); name != "" {
|
||||||
c.app.Name = name
|
c.app.Name = name
|
||||||
}
|
}
|
||||||
return Result{OK: true}
|
return Result{OK: true}
|
||||||
|
|
@ -138,7 +141,41 @@ func WithOptions(opts Options) CoreOption {
|
||||||
// core.WithService(display.Register(nil))
|
// core.WithService(display.Register(nil))
|
||||||
func WithService(factory func(*Core) Result) CoreOption {
|
func WithService(factory func(*Core) Result) CoreOption {
|
||||||
return func(c *Core) Result {
|
return func(c *Core) Result {
|
||||||
return factory(c)
|
r := factory(c)
|
||||||
|
if !r.OK {
|
||||||
|
return r
|
||||||
|
}
|
||||||
|
|
||||||
|
// If the factory returned a service instance, auto-discover and register
|
||||||
|
if r.Value != nil {
|
||||||
|
instance := r.Value
|
||||||
|
// Service name discovery from package path
|
||||||
|
typeOf := reflect.TypeOf(instance)
|
||||||
|
if typeOf.Kind() == reflect.Ptr {
|
||||||
|
typeOf = typeOf.Elem()
|
||||||
|
}
|
||||||
|
pkgPath := typeOf.PkgPath()
|
||||||
|
parts := Split(pkgPath, "/")
|
||||||
|
name := Lower(parts[len(parts)-1])
|
||||||
|
|
||||||
|
if name != "" {
|
||||||
|
// IPC handler discovery
|
||||||
|
instanceValue := reflect.ValueOf(instance)
|
||||||
|
handlerMethod := instanceValue.MethodByName("HandleIPCEvents")
|
||||||
|
if handlerMethod.IsValid() {
|
||||||
|
if handler, ok := handlerMethod.Interface().(func(*Core, Message) Result); ok {
|
||||||
|
c.RegisterAction(handler)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Register the service if not already registered by the factory
|
||||||
|
if sr := c.Service(name); !sr.OK {
|
||||||
|
c.Service(name, Service{})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return Result{OK: true}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
15
core_test.go
15
core_test.go
|
|
@ -64,6 +64,21 @@ func TestNew_WithServiceLock_Good(t *testing.T) {
|
||||||
assert.False(t, reg.OK)
|
assert.False(t, reg.OK)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestNew_WithService_Bad_FailingOption(t *testing.T) {
|
||||||
|
secondCalled := false
|
||||||
|
r := New(
|
||||||
|
WithService(func(c *Core) Result {
|
||||||
|
return Result{Value: E("test", "intentional failure", nil), OK: false}
|
||||||
|
}),
|
||||||
|
WithService(func(c *Core) Result {
|
||||||
|
secondCalled = true
|
||||||
|
return Result{OK: true}
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
assert.False(t, r.OK)
|
||||||
|
assert.False(t, secondCalled, "second option should not run after first fails")
|
||||||
|
}
|
||||||
|
|
||||||
// --- Accessors ---
|
// --- Accessors ---
|
||||||
|
|
||||||
func TestAccessors_Good(t *testing.T) {
|
func TestAccessors_Good(t *testing.T) {
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue