Skip to content

Commit

Permalink
Better names for CallNoPayload -> Get and CallWithPayload -> Fetch (#622
Browse files Browse the repository at this point in the history
)

Better names for CallNoPayload -> Get and CallWithPayload -> Fetch (and thus Fetch -> FetchBytes to free up that short name)
  • Loading branch information
ldemailly committed Sep 12, 2022
1 parent 83ce661 commit 0d61b4f
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 28 deletions.
2 changes: 1 addition & 1 deletion Makefile
Expand Up @@ -58,7 +58,7 @@ test: dependencies

local-lint:
golangci-lint version
golangci-lint $(DEBUG_LINTERS) run $(LINT_PACKAGES)
golangci-lint --timeout 120s $(DEBUG_LINTERS) run $(LINT_PACKAGES)

# Lint everything by default but ok to "make lint LINT_PACKAGES=./fhttp"
LINT_PACKAGES:=./...
Expand Down
12 changes: 6 additions & 6 deletions README.md
Expand Up @@ -52,13 +52,13 @@ You can install from source:
The [releases](https://github.com/fortio/fortio/releases) page has binaries for many OS/architecture combinations (see assets).

```shell
curl -L https://github.com/fortio/fortio/releases/download/v1.36.0/fortio-linux_amd64-1.36.0.tgz \
curl -L https://github.com/fortio/fortio/releases/download/v1.37.0/fortio-linux_amd64-1.37.0.tgz \
| sudo tar -C / -xvzpf -
# or the debian package
wget https://github.com/fortio/fortio/releases/download/v1.36.0/fortio_1.36.0_amd64.deb
dpkg -i fortio_1.36.0_amd64.deb
wget https://github.com/fortio/fortio/releases/download/v1.37.0/fortio_1.37.0_amd64.deb
dpkg -i fortio_1.37.0_amd64.deb
# or the rpm
rpm -i https://github.com/fortio/fortio/releases/download/v1.36.0/fortio-1.36.0-1.x86_64.rpm
rpm -i https://github.com/fortio/fortio/releases/download/v1.37.0/fortio-1.37.0-1.x86_64.rpm
# and more, see assets in release page
```

Expand All @@ -68,7 +68,7 @@ On a MacOS you can also install Fortio using [Homebrew](https://brew.sh/):
brew install fortio
```

On Windows, download https://github.com/fortio/fortio/releases/download/v1.36.0/fortio_win_1.36.0.zip and extract `fortio.exe` to any location, then using the Windows Command Prompt:
On Windows, download https://github.com/fortio/fortio/releases/download/v1.37.0/fortio_win_1.37.0.zip and extract `fortio.exe` to any location, then using the Windows Command Prompt:
```
fortio.exe server
```
Expand Down Expand Up @@ -116,7 +116,7 @@ Full list of command line flags (`fortio help`):
<details>
<!-- use release/updateFlags.sh to update this section -->
<pre>
Φορτίο 1.36.0 usage:
Φορτίο 1.37.0 usage:
fortio command [flags] target
where command is one of: load (load testing), server (starts ui, rest api,
http-echo, redirect, proxies, tcp-echo and grpc ping servers), tcp-echo (only
Expand Down
39 changes: 31 additions & 8 deletions jrpc/jrpcClient.go
Expand Up @@ -18,6 +18,11 @@ package jrpc // import "fortio.org/fortio/jrpc"

// This package is a true self contained library, that doesn't rely on our logger nor other packages
// in fortio/ outside of version/ (which now also doesn't rely on logger or any other package).
// Naming is hard, we have Call, Send, Get, Fetch and FetchBytes pretty much all meaning retrieving data
// from a URL with the variants depending on whether we have something to serialize and if it's bytes
// or struct based in and out. Additionally *URL() variants are for when no additional headers or options
// are needed and the url is just a plain string. If golang supported multiple signatures it would be a single
// method name instead of 8.
import (
"bytes"
"context"
Expand Down Expand Up @@ -91,22 +96,29 @@ func Call[Q any, T any](url *Destination, payload *T) (*Q, error) {
return nil, err
}
}
return CallWithPayload[Q](url, bytes)
return Fetch[Q](url, bytes)
}

// CallURL is Call without any options/non default headers, timeout etc and just the URL.
func CallURL[Q any, T any](url string, payload *T) (*Q, error) {
return Call[Q](NewDestination(url), payload)
}

// CallNoPayload is for an API call without json payload.
// Deprecated: CallNoPayload is for an API call without json payload.
// Use Get() instead.
func CallNoPayload[Q any](url *Destination) (*Q, error) {
return CallWithPayload[Q](url, []byte{})
return Get[Q](url)
}

// CallNoPayloadURL short cut for CallNoPayload with url as a string (default Send()/Destination options).
func CallNoPayloadURL[Q any](url string) (*Q, error) {
return CallWithPayload[Q](NewDestination(url), []byte{})
// Get fetches and deseializes the JSON returned by the Destination into a Q struct.
// Used when there is no json payload to send.
func Get[Q any](url *Destination) (*Q, error) {
return Fetch[Q](url, []byte{})
}

// GetURL is Get without additional options (default timeout and headers).
func GetURL[Q any](url string) (*Q, error) {
return Get[Q](NewDestination(url))
}

// Serialize serializes the object as json.
Expand All @@ -121,8 +133,16 @@ func Deserialize[Q any](bytes []byte) (*Q, error) {
return &result, err // Will return zero object, not nil upon error
}

// CallWithPayload is for cases where the payload is already serialized (or empty).
// Deprecated: CallWithPayload use Fetch() instead.
func CallWithPayload[Q any](url *Destination, bytes []byte) (*Q, error) {
return Fetch[Q](url, bytes)
}

// Fetch is for cases where the payload is already serialized (or empty
// but call Get() when empty for clarity).
// Note that if you're looking for the []byte version instead of this
// generics version, it's now called FetchBytes().
func Fetch[Q any](url *Destination, bytes []byte) (*Q, error) {
code, bytes, err := Send(url, bytes) // returns -1 on other errors
if err != nil {
return nil, err
Expand Down Expand Up @@ -197,12 +217,15 @@ func NewDestination(url string) *Destination {
}

// FetchURL is Send without a payload and no additional options (default timeout and headers).
// Technically this should be called FetchBytesURL().
func FetchURL(url string) (int, []byte, error) {
return Send(NewDestination(url), []byte{})
}

// Fetch is Send without a payload (so will be a GET request).
func Fetch(url *Destination) (int, []byte, error) {
// Used to be called Fetch() but we needed that shorter name to
// simplify the former CallWithPayload function name.
func FetchBytes(url *Destination) (int, []byte, error) {
return Send(url, []byte{})
}

Expand Down
11 changes: 6 additions & 5 deletions jrpc/jrpc_test.go
Expand Up @@ -144,8 +144,8 @@ func TestJPRC(t *testing.T) {
t.Errorf("response doesn't contain expected string: %+v", res)
}
// Error cases
// Empty request, using Fetch()
code, bytes, err := jrpc.Fetch(jrpc.NewDestination(url))
// Empty request, using FetchBytes()
code, bytes, err := jrpc.FetchBytes(jrpc.NewDestination(url))
if err != nil {
t.Errorf("failed Fetch: %v - %s", err, jrpc.DebugSummary(bytes, 256))
}
Expand Down Expand Up @@ -176,7 +176,7 @@ func TestJPRC(t *testing.T) {
if de != nil && !strings.HasPrefix(de.Error(), expected) {
t.Errorf("expected dns error to start with %q, got %q", expected, de.Error())
}
// bad json payload sent
// bad json payload sent - call deprecated one for coverage for now, replace with Fetch() when it's gone:
errReply, err := jrpc.CallWithPayload[Response](jrpc.NewDestination(url), []byte(`{foo: missing-quotes}`))
if err == nil {
t.Errorf("expected error, got nil and %v", res)
Expand All @@ -195,8 +195,8 @@ func TestJPRC(t *testing.T) {
if errReply.Exception != expected {
t.Errorf("expected Exception in body to be %q, got %+v", expected, errReply)
}
// bad json response, using Fetch()
errReply, err = jrpc.CallNoPayloadURL[Response](url)
// bad json response, using GetURL()
errReply, err = jrpc.GetURL[Response](url)
if err == nil {
t.Errorf("expected error %v", errReply)
}
Expand Down Expand Up @@ -310,6 +310,7 @@ func TestJPRCHeaders(t *testing.T) {
URL: url,
Headers: &inp,
}
// use the deprecated for coverage. switch to jrpc.Get() in next version
res, err := jrpc.CallNoPayload[http.Header](dest)
if err != nil {
t.Errorf("failed Call: %v", err)
Expand Down
16 changes: 8 additions & 8 deletions rapi/restHandler_test.go
Expand Up @@ -37,7 +37,7 @@ import (

// Generics ftw.
func FetchResult[T any](t *testing.T, url string, jsonPayload string) *T {
r, err := jrpc.CallWithPayload[T](jrpc.NewDestination(url), []byte(jsonPayload))
r, err := jrpc.Fetch[T](jrpc.NewDestination(url), []byte(jsonPayload))
if err != nil {
t.Errorf("Got unexpected error for URL %s: %v - %v", url, err, r)
}
Expand All @@ -63,7 +63,7 @@ func GetAsyncResult(t *testing.T, url string, jsonPayload string) *AsyncReply {

// Same as above but when expecting to get an error reply.
func GetErrorResult(t *testing.T, url string, jsonPayload string) *jrpc.ServerReply {
r, err := jrpc.CallWithPayload[jrpc.ServerReply](jrpc.NewDestination(url), []byte(jsonPayload))
r, err := jrpc.Fetch[jrpc.ServerReply](jrpc.NewDestination(url), []byte(jsonPayload))
if err == nil {
t.Errorf("Got unexpected no error for URL %s: %v", url, r)
}
Expand Down Expand Up @@ -199,7 +199,7 @@ func TestHTTPRunnerRESTApi(t *testing.T) {
URL: statusURL,
Timeout: 3 * time.Second,
}
statuses, err := jrpc.CallNoPayload[StatusReply](statusDest)
statuses, err := jrpc.Get[StatusReply](statusDest)
if err != nil {
t.Errorf("Error getting status %q: %v", statusURL, err)
}
Expand Down Expand Up @@ -259,7 +259,7 @@ func TestHTTPRunnerRESTApi(t *testing.T) {
t.Errorf("2nd stop should be noop, got %+v", asyncObj)
}
// Status should be empty (nothing running)
statuses, err = jrpc.CallNoPayload[StatusReply](statusDest)
statuses, err = jrpc.Get[StatusReply](statusDest)
if err != nil {
t.Errorf("Error getting status %q: %v", statusURL, err)
}
Expand All @@ -274,7 +274,7 @@ func TestHTTPRunnerRESTApi(t *testing.T) {
// Get all statuses
statusURL = fmt.Sprintf("http://localhost:%d%s%s", addr.Port, uiPath, RestStatusURI)
statusDest.URL = statusURL
statuses, err = jrpc.CallNoPayload[StatusReply](statusDest)
statuses, err = jrpc.Get[StatusReply](statusDest)
if err != nil {
t.Errorf("Error getting status %q: %v", statusURL, err)
}
Expand Down Expand Up @@ -376,7 +376,7 @@ func TestRESTStopTimeBased(t *testing.T) {
// Get status
statusURL := fmt.Sprintf("http://localhost:%d%s%s?runid=%d", addr.Port, uiPath, RestStatusURI, runID)
statusDest := jrpc.NewDestination(statusURL)
statuses, err := jrpc.CallNoPayload[StatusReply](statusDest)
statuses, err := jrpc.Get[StatusReply](statusDest)
if err != nil {
t.Errorf("Error getting status %q: %v", statusURL, err)
}
Expand Down Expand Up @@ -418,7 +418,7 @@ func TestRESTStopTimeBased(t *testing.T) {
t.Errorf("2nd stop should be noop, got %+v", asyncObj)
}
// Status should be empty (nothing running)
statuses, err = jrpc.CallNoPayload[StatusReply](statusDest)
statuses, err = jrpc.Get[StatusReply](statusDest)
if err != nil {
t.Errorf("Error getting status %q: %v", statusURL, err)
}
Expand Down Expand Up @@ -475,7 +475,7 @@ func TestRESTStopTimeBased(t *testing.T) {

// If jsonPayload isn't empty we POST otherwise get the url.
func GetGRPCResult(t *testing.T, url string, jsonPayload string) *fgrpc.GRPCRunnerResults {
r, err := jrpc.CallWithPayload[fgrpc.GRPCRunnerResults](jrpc.NewDestination(url), []byte(jsonPayload))
r, err := jrpc.Fetch[fgrpc.GRPCRunnerResults](jrpc.NewDestination(url), []byte(jsonPayload))
if err != nil {
t.Errorf("Got unexpected err for URL %s: %v", url, err)
}
Expand Down

0 comments on commit 0d61b4f

Please sign in to comment.