Compare commits

..

1 Commits

Author SHA1 Message Date
69e2c873d8 Refactor for interface clarity and dependency isolation.
Some checks failed
Go / build-and-release (push) Has been cancelled
Replaced inline interface literals with dedicated, documented interface definitions in `pkg/interfaces/`. Introduced `TimeoutError`, `PolicyChecker`, and `Neo4jResultIterator` interfaces to clarify design, improve maintainability, and resolve potential circular dependencies. Updated config and constant usage rules for consistency. Incremented version to v0.31.11.
2025-12-03 06:04:50 +00:00
11 changed files with 177 additions and 23 deletions

View File

@@ -82,6 +82,49 @@ func (f *File) Read(p []byte) (n int, err error) {
} }
``` ```
### Interface Design - CRITICAL RULES
**Rule 1: Define interfaces in a dedicated package (e.g., `pkg/interfaces/<name>/`)**
- Interfaces provide isolation between packages and enable dependency inversion
- Keeping interfaces in a dedicated package prevents circular dependencies
- Each interface package should be minimal (just the interface, no implementations)
**Rule 2: NEVER use type assertions with interface literals**
- **NEVER** write `.(interface{ Method() Type })` - this is non-idiomatic and unmaintainable
- Interface literals cannot be documented, tested for satisfaction, or reused
```go
// BAD - interface literal in type assertion (NEVER DO THIS)
if checker, ok := obj.(interface{ Check() bool }); ok {
checker.Check()
}
// GOOD - use defined interface from dedicated package
import "myproject/pkg/interfaces/checker"
if c, ok := obj.(checker.Checker); ok {
c.Check()
}
```
**Rule 3: Resolving Circular Dependencies**
- If a circular dependency occurs, move the interface to `pkg/interfaces/`
- The implementing type stays in its original package
- The consuming code imports only the interface package
- Pattern:
```
pkg/interfaces/foo/ <- interface definition (no dependencies)
↑ ↑
pkg/bar/ pkg/baz/
(implements) (consumes via interface)
```
**Rule 4: Verify interface satisfaction at compile time**
```go
// Add this line to ensure *MyType implements MyInterface
var _ MyInterface = (*MyType)(nil)
```
### Concurrency ### Concurrency
Use goroutines and channels for concurrent programming: Use goroutines and channels for concurrent programming:
@@ -178,6 +221,26 @@ For detailed information, consult the reference files:
- Start comments with the name being described - Start comments with the name being described
- Use godoc format - Use godoc format
6. **Configuration - CRITICAL**
- **NEVER** use `os.Getenv()` scattered throughout packages
- **ALWAYS** centralize environment variable parsing in a single config package (e.g., `app/config/`)
- Pass configuration via structs, not by reading environment directly
- This ensures discoverability, documentation, and testability of all config options
7. **Constants - CRITICAL**
- **ALWAYS** define named constants for values used more than a few times
- **ALWAYS** define named constants if multiple packages depend on the same value
- Constants shared across packages belong in a dedicated package (e.g., `pkg/constants/`)
- Magic numbers and strings are forbidden
```go
// BAD - magic number
if size > 1024 {
// GOOD - named constant
const MaxBufferSize = 1024
if size > MaxBufferSize {
```
## Common Commands ## Common Commands
```bash ```bash

View File

@@ -346,10 +346,11 @@ export ORLY_AUTH_TO_WRITE=false # Require auth only for writes
4. Events stored via `database.SaveEvent()` 4. Events stored via `database.SaveEvent()`
5. Active subscriptions notified via `publishers.Publish()` 5. Active subscriptions notified via `publishers.Publish()`
**Configuration System:** **Configuration System - CRITICAL RULES:**
- Uses `go-simpler.org/env` for struct tags - Uses `go-simpler.org/env` for struct tags
- **IMPORTANT: ALL environment variables MUST be defined in `app/config/config.go`** - **ALL environment variables MUST be defined in `app/config/config.go`**
- Never use `os.Getenv()` directly in packages - always pass config via structs - **NEVER** use `os.Getenv()` directly in packages - always pass config via structs
- **NEVER** parse environment variables outside of `app/config/`
- This ensures all config options appear in `./orly help` output - This ensures all config options appear in `./orly help` output
- Database backends receive config via `database.DatabaseConfig` struct - Database backends receive config via `database.DatabaseConfig` struct
- Use `GetDatabaseConfigValues()` helper to extract DB config from app config - Use `GetDatabaseConfigValues()` helper to extract DB config from app config
@@ -358,6 +359,21 @@ export ORLY_AUTH_TO_WRITE=false # Require auth only for writes
- Default data directory: `~/.local/share/ORLY` - Default data directory: `~/.local/share/ORLY`
- Database-specific config (Neo4j, DGraph, Badger) is passed via `DatabaseConfig` struct in `pkg/database/factory.go` - Database-specific config (Neo4j, DGraph, Badger) is passed via `DatabaseConfig` struct in `pkg/database/factory.go`
**Constants - CRITICAL RULES:**
- **ALWAYS** define named constants for values used more than a few times
- **ALWAYS** define named constants if multiple packages depend on the same value
- Constants shared across packages should be in a dedicated package (e.g., `pkg/constants/`)
- Magic numbers and strings are forbidden - use named constants with clear documentation
- Example:
```go
// BAD - magic number
if timeout > 30 {
// GOOD - named constant
const DefaultTimeoutSeconds = 30
if timeout > DefaultTimeoutSeconds {
```
**Event Publishing:** **Event Publishing:**
- `pkg/protocol/publish/` manages publisher registry - `pkg/protocol/publish/` manages publisher registry
- Each WebSocket connection registers its subscriptions - Each WebSocket connection registers its subscriptions
@@ -394,6 +410,53 @@ export ORLY_AUTH_TO_WRITE=false # Require auth only for writes
``` ```
- This optimization saves memory and enables faster comparisons in the database layer - This optimization saves memory and enables faster comparisons in the database layer
**Interface Design - CRITICAL RULES:**
**Rule 1: ALL interfaces MUST be defined in `pkg/interfaces/<name>/`**
- Interfaces provide isolation between packages and enable dependency inversion
- Keeping interfaces in a dedicated package prevents circular dependencies
- Each interface package should be minimal (just the interface, no implementations)
**Rule 2: NEVER use type assertions with interface literals**
- **NEVER** write `.(interface{ Method() Type })` - this is non-idiomatic and unmaintainable
- Interface literals cannot be documented, tested for satisfaction, or reused
- Example of WRONG approach:
```go
// BAD - interface literal in type assertion
if checker, ok := obj.(interface{ Check() bool }); ok {
checker.Check()
}
```
- Example of CORRECT approach:
```go
// GOOD - use defined interface from pkg/interfaces/
import "next.orly.dev/pkg/interfaces/checker"
if c, ok := obj.(checker.Checker); ok {
c.Check()
}
```
**Rule 3: Resolving Circular Dependencies**
- If a circular dependency occurs when adding an interface, move the interface to `pkg/interfaces/`
- The implementing type stays in its original package
- The consuming code imports only the interface package
- This pattern:
```
pkg/interfaces/foo/ <- interface definition (no dependencies)
↑ ↑
pkg/bar/ pkg/baz/
(implements) (consumes via interface)
```
**Existing interfaces in `pkg/interfaces/`:**
- `acl/` - ACL and PolicyChecker interfaces
- `neterr/` - TimeoutError interface for network errors
- `resultiter/` - Neo4jResultIterator for database results
- `store/` - Storage-related interfaces
- `publisher/` - Event publishing interfaces
- `typer/` - Type identification interface
## Development Workflow ## Development Workflow
### Making Changes to Web UI ### Making Changes to Web UI

View File

@@ -12,6 +12,7 @@ import (
"time" "time"
"github.com/gorilla/websocket" "github.com/gorilla/websocket"
"next.orly.dev/pkg/interfaces/neterr"
) )
var ( var (
@@ -90,7 +91,7 @@ func main() {
if ctx.Err() != nil { if ctx.Err() != nil {
return return
} }
if netErr, ok := err.(interface{ Timeout() bool }); ok && netErr.Timeout() { if netErr, ok := err.(neterr.TimeoutError); ok && netErr.Timeout() {
continue continue
} }
log.Printf("Read error: %v", err) log.Printf("Read error: %v", err)

View File

@@ -13,6 +13,7 @@ import (
"time" "time"
"github.com/gorilla/websocket" "github.com/gorilla/websocket"
"next.orly.dev/pkg/interfaces/neterr"
) )
var ( var (
@@ -123,7 +124,7 @@ func main() {
} }
// Check for timeout errors (these are expected during idle periods) // Check for timeout errors (these are expected during idle periods)
if netErr, ok := err.(interface{ Timeout() bool }); ok && netErr.Timeout() { if netErr, ok := err.(neterr.TimeoutError); ok && netErr.Timeout() {
consecutiveTimeouts++ consecutiveTimeouts++
if consecutiveTimeouts >= maxConsecutiveTimeouts { if consecutiveTimeouts >= maxConsecutiveTimeouts {
log.Printf("Too many consecutive read timeouts (%d), connection may be dead", consecutiveTimeouts) log.Printf("Too many consecutive read timeouts (%d), connection may be dead", consecutiveTimeouts)

View File

@@ -2,20 +2,20 @@ package acl
import ( import (
"git.mleku.dev/mleku/nostr/encoders/event" "git.mleku.dev/mleku/nostr/encoders/event"
"next.orly.dev/pkg/interfaces/acl" acliface "next.orly.dev/pkg/interfaces/acl"
"next.orly.dev/pkg/utils/atomic" "next.orly.dev/pkg/utils/atomic"
) )
var Registry = &S{} var Registry = &S{}
type S struct { type S struct {
ACL []acl.I ACL []acliface.I
Active atomic.String Active atomic.String
} }
type A struct{ S } type A struct{ S }
func (s *S) Register(i acl.I) { func (s *S) Register(i acliface.I) {
(*s).ACL = append((*s).ACL, i) (*s).ACL = append((*s).ACL, i)
} }
@@ -85,9 +85,7 @@ func (s *S) CheckPolicy(ev *event.E) (allowed bool, err error) {
for _, i := range s.ACL { for _, i := range s.ACL {
if i.Type() == s.Active.Load() { if i.Type() == s.Active.Load() {
// Check if the ACL implementation has a CheckPolicy method // Check if the ACL implementation has a CheckPolicy method
if policyChecker, ok := i.(interface { if policyChecker, ok := i.(acliface.PolicyChecker); ok {
CheckPolicy(ev *event.E) (allowed bool, err error)
}); ok {
return policyChecker.CheckPolicy(ev) return policyChecker.CheckPolicy(ev)
} }
// If no CheckPolicy method, default to allowing // If no CheckPolicy method, default to allowing

View File

@@ -2,6 +2,7 @@
package acl package acl
import ( import (
"git.mleku.dev/mleku/nostr/encoders/event"
"next.orly.dev/pkg/interfaces/typer" "next.orly.dev/pkg/interfaces/typer"
) )
@@ -31,3 +32,9 @@ type I interface {
Syncer() Syncer()
typer.T typer.T
} }
// PolicyChecker is an optional interface that ACL implementations can implement
// to provide custom event policy checking beyond basic access level checks.
type PolicyChecker interface {
CheckPolicy(ev *event.E) (allowed bool, err error)
}

View File

@@ -0,0 +1,8 @@
// Package neterr defines interfaces for network error handling.
package neterr
// TimeoutError is an interface for errors that can indicate a timeout.
// This is compatible with net.Error's Timeout() method.
type TimeoutError interface {
Timeout() bool
}

View File

@@ -0,0 +1,16 @@
// Package resultiter defines interfaces for iterating over database query results.
package resultiter
import (
"context"
"github.com/neo4j/neo4j-go-driver/v5/neo4j"
)
// Neo4jResultIterator defines the interface for iterating over Neo4j query results.
// This is implemented by both neo4j.Result and CollectedResult types.
type Neo4jResultIterator interface {
Next(context.Context) bool
Record() *neo4j.Record
Err() error
}

View File

@@ -5,12 +5,12 @@ import (
"fmt" "fmt"
"strings" "strings"
"github.com/neo4j/neo4j-go-driver/v5/neo4j"
"next.orly.dev/pkg/database/indexes/types"
"git.mleku.dev/mleku/nostr/encoders/event" "git.mleku.dev/mleku/nostr/encoders/event"
"git.mleku.dev/mleku/nostr/encoders/filter" "git.mleku.dev/mleku/nostr/encoders/filter"
"git.mleku.dev/mleku/nostr/encoders/hex" "git.mleku.dev/mleku/nostr/encoders/hex"
"git.mleku.dev/mleku/nostr/encoders/tag" "git.mleku.dev/mleku/nostr/encoders/tag"
"next.orly.dev/pkg/database/indexes/types"
"next.orly.dev/pkg/interfaces/resultiter"
"next.orly.dev/pkg/interfaces/store" "next.orly.dev/pkg/interfaces/store"
) )
@@ -186,14 +186,10 @@ func (n *N) parseEventsFromResult(result any) ([]*event.E, error) {
events := make([]*event.E, 0) events := make([]*event.E, 0)
ctx := context.Background() ctx := context.Background()
// Type assert to the interface we actually use // Type assert to the result iterator interface
resultIter, ok := result.(interface { resultIter, ok := result.(resultiter.Neo4jResultIterator)
Next(context.Context) bool
Record() *neo4j.Record
Err() error
})
if !ok { if !ok {
return nil, fmt.Errorf("invalid result type") return nil, fmt.Errorf("invalid result type: expected resultiter.Neo4jResultIterator")
} }
// Iterate through result records // Iterate through result records

View File

@@ -1 +1 @@
v0.31.10 v0.31.11

View File

@@ -7,9 +7,10 @@ import (
"time" "time"
"github.com/gorilla/websocket" "github.com/gorilla/websocket"
"lol.mleku.dev/errorf"
"git.mleku.dev/mleku/nostr/encoders/event" "git.mleku.dev/mleku/nostr/encoders/event"
"git.mleku.dev/mleku/nostr/encoders/hex" "git.mleku.dev/mleku/nostr/encoders/hex"
"lol.mleku.dev/errorf"
"next.orly.dev/pkg/interfaces/neterr"
) )
// Client wraps a WebSocket connection to a relay for testing. // Client wraps a WebSocket connection to a relay for testing.
@@ -118,7 +119,7 @@ func (c *Client) readLoop() {
default: default:
} }
// Check if it's a timeout - connection might still be alive // Check if it's a timeout - connection might still be alive
if netErr, ok := err.(interface{ Timeout() bool }); ok && netErr.Timeout() { if netErr, ok := err.(neterr.TimeoutError); ok && netErr.Timeout() {
// Pong handler should have extended deadline, but if we timeout, // Pong handler should have extended deadline, but if we timeout,
// reset it and continue - connection might still be alive // reset it and continue - connection might still be alive
// This can happen during idle periods when no messages are received // This can happen during idle periods when no messages are received