feat: method namer #130

chris-4chain · 2025-03-17T06:51:47Z

I'm proposing a MethodNamer custom configuration option (for both client & server) to make it configurable what string is sent as method field in JSON-RPC.

Definition:

// MethodNamer is a function that takes a namespace and a method name and returns the full method name, sent via JSON-RPC.
// This is useful if you want to customize the default behaviour, e.g. send without the namespace or make it lowercase.
type MethodNamer func(string, string) string

Example:

// given a server:
rpcServer := jsonrpc.NewServer(jsonrpc.WithServerMethodNamer(jsonrpc.NoNamespaceMethodNamer))
serverHandler := &SimpleServerHandler{}
rpcServer.Register("SimpleServerHandler", serverHandler)

testServ := httptest.NewServer(rpcServer)
defer testServ.Close()

// and a client:
var client struct {
    AddGet func(int) int
}
closer, _:= jsonrpc.NewMergeClient(
	context.Background(),
	"http://"+testServ.Listener.Addr().String(),
	"SimpleServerHandler",
	[]any{&client},
	nil,
	jsonrpc.WithMethodNamer(jsonrpc.NoNamespaceMethodNamer),
)
defer closer()

// when:
n := client.AddGet(123)

// then:
// an RPC request should be sent with "AddGet" as "method" (without namespace)
// n should == 123

It solves issues: Add support for lowercase methods and Add support for no namespace

I've provided most common (I guess) namers:
- NoNamespaceMethodNamer - returns the method name as is, without the namespace.
- NoNamespaceDecapitalizedMethodNamer - returns the method name as is, without the namespace, and decapitalizes the first letter.
I've added additional tests.
All tests pass (locally, 64bit computer)

chris-4chain · 2025-03-17T07:12:13Z

On 32 bit machine, I can see that the tests fail, but, I suppose, it is not related to my changes 🤔

It's rather related to atomic package: https://pkg.go.dev/sync/atomic#pkg-note-BUG
They suggest to use Int64.Add instead of atomic.AddInt64: https://pkg.go.dev/sync/atomic#AddInt64

BigLep · 2025-03-18T02:21:41Z

Thanks @chris-4chain for your contribution.

I'll let @rvagg chime in on next steps or assign another reviewer.

Stebalien · 2025-03-21T17:55:42Z

Gah. Sorry, I saw and merged #131 first, suggesting your exact design. However, that PR doesn't implement the client side. Mind rebasing your PR?

add options to set custom namespace separator and a method name trans…

chris-4chain · 2025-03-24T10:23:04Z

@Stebalien No worries 👍 I'm glad to see this feature moving forward, regardless of who creates the PR 🙈

I've adjusted my code and added the client-side (MethodNameFormatter)
I've also adjusted the name "MethodNamer" (proposed by me) to the one, already merged ("MethodNameFormatter")
I've changed the option function for server from WithMethodNameFormatter to WithServerMethodNameFormatter (for consistency reasons). The WithMethodNameFormatter is now for the client side.
The issue with 32 bit is resolved - I've changed an order of fields in the client struct - it helped 🤞

rvagg · 2025-04-01T04:42:36Z

README.md

+There are three predefined formatters:
+- `jsonrpc.DefaultMethodNameFormatter` - default method name formatter, e.g. `SimpleServerHandler.AddGet`
+- `jsonrpc.NoNamespaceMethodNameFormatter` - method name formatter without namespace, e.g. `AddGet`
+- `jsonrpc.NoNamespaceDecapitalizedMethodNameFormatter` - method name formatter without namespace and decapitalized, e.g. `addGet`


I don't love this name, but NoNamespaceLowerCamelCaseMethodNameFormatter would be getting a bit out of hand I think

✅ I've adjusted it according to your idea with NewMethodNameFormatter

client.go

rvagg · 2025-04-01T04:45:12Z

client.go

+		methodNameFormatter: config.methodNamer,
+		paramEncoders:       config.paramEncoders,
+		errors:              config.errors,


Suggested change

methodNameFormatter: config.methodNamer,

paramEncoders: config.paramEncoders,

errors: config.errors,

paramEncoders: config.paramEncoders,

errors: config.errors,

methodNameFormatter: config.methodNamer,

retain ordering of the fields as defined

rvagg · 2025-04-01T04:45:18Z

client.go

-		paramEncoders: config.paramEncoders,
-		errors:        config.errors,
+		namespace:           namespace,
+		methodNameFormatter: config.methodNamer,


rvagg · 2025-04-01T04:45:23Z

client.go

-		paramEncoders: config.paramEncoders,
-		errors:        config.errors,
+		namespace:           namespace,
+		methodNameFormatter: config.methodNamer,


rvagg · 2025-04-01T04:57:53Z

method_formatter.go

+// DefaultMethodNameFormatter joins the namespace and method name with a dot.
+func DefaultMethodNameFormatter(namespace, method string) string {
+	return namespace + "." + method
+}
+
+// NoNamespaceMethodNameFormatter returns the method name as is, without the namespace.
+func NoNamespaceMethodNameFormatter(_, method string) string {
+	return method
+}
+
+// NoNamespaceDecapitalizedMethodNameFormatter returns the method name as is, without the namespace, and decapitalizes the first letter.
+// e.g. "Inc" -> "inc"
+func NoNamespaceDecapitalizedMethodNameFormatter(_, method string) string {
+	if len(method) == 0 {
+		return ""
+	}
+	return strings.ToLower(method[:1]) + method[1:]
+}


Suggested change

// DefaultMethodNameFormatter joins the namespace and method name with a dot.

func DefaultMethodNameFormatter(namespace, method string) string {

return namespace + "." + method

}

// NoNamespaceMethodNameFormatter returns the method name as is, without the namespace.

func NoNamespaceMethodNameFormatter(_, method string) string {

return method

}

// NoNamespaceDecapitalizedMethodNameFormatter returns the method name as is, without the namespace, and decapitalizes the first letter.

// e.g. "Inc" -> "inc"

func NoNamespaceDecapitalizedMethodNameFormatter(_, method string) string {

if len(method) == 0 {

return ""

}

return strings.ToLower(method[:1]) + method[1:]

}

// CaseStyle represents the case style for method names.

type CaseStyle int

const (

OriginalCase CaseStyle = iota

LowerFirstCharCase

)

// NewMethodNameFormatter creates a new method name formatter based on the provided options.

func NewMethodNameFormatter(includeNamespace bool, nameCase CaseStyle) MethodNameFormatter {

return func(namespace, method string) string {

formattedMethod := method

if nameCase == LowerFirstCharCase && len(method) > 0 {

formattedMethod = strings.ToLower(method[:1]) + method[1:]

}

if includeNamespace {

return namespace + "." + formattedMethod

}

return formattedMethod

}

}

// DefaultMethodNameFormatter is a pass-through formatter with default options.

var DefaultMethodNameFormatter = NewMethodNameFormatter(true, OriginalCase)

How about something like this to solve the weird naming problem?

Good idea 💯

README.md

rvagg

great, thanks! sorry it's taken so long on our end

BigLep requested a review from rvagg March 18, 2025 02:21

BigLep added this to FilOz Mar 18, 2025

github-project-automation bot moved this to 📌 Triage in FilOz Mar 18, 2025

BigLep moved this from 📌 Triage to 🔎 Awaiting Review in FilOz Mar 18, 2025

Stebalien mentioned this pull request Mar 21, 2025

chore: release v0.8.0 #132

Closed

chris-4chain added 3 commits March 24, 2025 09:40

Merge pull request #1 from filecoin-project/master

15eee17

add options to set custom namespace separator and a method name trans…

feat: method namer

fe5d436

feat: adjust to MethodNameFormatter concept

57d1697

chris-4chain force-pushed the method-namer branch from cb17873 to 57d1697 Compare March 24, 2025 10:09

rvagg reviewed Apr 1, 2025

View reviewed changes

client.go Show resolved Hide resolved

rvagg reviewed Apr 1, 2025

View reviewed changes

chris-4chain added 2 commits April 1, 2025 09:12

feat: NewMethodNameFormatter concept

a86ba49

feat: client instantiation ordering

19da84b

chris-4chain requested a review from rvagg April 1, 2025 07:43

rvagg reviewed Apr 8, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

Update README.md

e72f58d

rvagg approved these changes Apr 8, 2025

View reviewed changes

github-project-automation bot moved this from 🔎 Awaiting Review to ✔️ Approved by reviewer in FilOz Apr 8, 2025

rvagg merged commit e691565 into filecoin-project:master Apr 8, 2025
7 checks passed

github-project-automation bot moved this from ✔️ Approved by reviewer to 🎉 Done in FilOz Apr 8, 2025

rvagg mentioned this pull request Jul 29, 2025

Websocket Subscription support #136

Open

-// DefaultMethodNameFormatter joins the namespace and method name with a dot.
-func DefaultMethodNameFormatter(namespace, method string) string {
-	return namespace + "." + method
-}
-// NoNamespaceMethodNameFormatter returns the method name as is, without the namespace.
-func NoNamespaceMethodNameFormatter(_, method string) string {
-	return method
-}
-// NoNamespaceDecapitalizedMethodNameFormatter returns the method name as is, without the namespace, and decapitalizes the first letter.
-// e.g. "Inc" -> "inc"
-func NoNamespaceDecapitalizedMethodNameFormatter(_, method string) string {
-	if len(method) == 0 {
-		return ""
-	}
-	return strings.ToLower(method[:1]) + method[1:]
-}
+// CaseStyle represents the case style for method names.
+type CaseStyle int
+const (
+    OriginalCase CaseStyle = iota
+    LowerFirstCharCase
+)
+// NewMethodNameFormatter creates a new method name formatter based on the provided options.
+func NewMethodNameFormatter(includeNamespace bool, nameCase  CaseStyle) MethodNameFormatter {
+    return func(namespace, method string) string {
+        formattedMethod := method
+        if nameCase == LowerFirstCharCase && len(method) > 0 {
+            formattedMethod = strings.ToLower(method[:1]) + method[1:]
+        }
+        if includeNamespace {
+            return namespace + "." + formattedMethod
+        }
+        return formattedMethod
+    }
+}
+// DefaultMethodNameFormatter is a pass-through formatter with default options.
+var DefaultMethodNameFormatter = NewMethodNameFormatter(true, OriginalCase)

feat: method namer #130

feat: method namer #130

Uh oh!

Conversation

chris-4chain commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chris-4chain commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BigLep commented Mar 18, 2025

Uh oh!

Stebalien commented Mar 21, 2025

Uh oh!

chris-4chain commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chris-4chain Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rvagg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chris-4chain commented Mar 17, 2025 •

edited

Loading

chris-4chain commented Mar 17, 2025 •

edited

Loading

chris-4chain commented Mar 24, 2025 •

edited

Loading

chris-4chain Apr 1, 2025 •

edited

Loading