Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better names for CallNoPayload -> Get and CallWithPayload -> Fetch #622

Merged
merged 3 commits into from Sep 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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