Defined lbryinc.Caller interface #63

Closed
anbsky wants to merge 1 commit from feature/lbryinc_client_caller into master
2 changed files with 34 additions and 10 deletions

View file

@ -13,11 +13,18 @@ import (
log "github.com/sirupsen/logrus"
)
// Client stores data about internal-apis call it is about to make.
// Caller interface defines currently supported internal-apis methods.
type Caller interface {
SetServerAddress(string)
tiger5226 commented 2019-07-14 03:54:26 +02:00 (Migrated from github.com)
Review

So when naming interfaces, those that end with an er should only have one method and it should be that remaining string. This is the convention for single method interfaces. So if you are naming an interface Caller it should only specify one method and it should be called Call.

So when naming interfaces, those that end with an `er` should only have one method and it should be that remaining string. This is the convention for single method interfaces. So if you are naming an interface `Caller` it should only specify one method and it should be called `Call`.
lyoshenka commented 2019-07-15 15:11:24 +02:00 (Migrated from github.com)
Review

I did not know this

I did not know this
tiger5226 commented 2019-07-16 03:55:20 +02:00 (Migrated from github.com)
Review
https://golang.org/doc/effective_go.html#interface-names for reference
GetServerAddress() string
UserMe() (ResponseData, error)
}
tiger5226 commented 2019-07-14 03:49:30 +02:00 (Migrated from github.com)
Review

ResponseData is a bad name. It should return UserMeResponse to conform to how we handle other clients with go. So that no fields of the response need to be parsed after the client has returned the response.

`ResponseData` is a bad name. It should return `UserMeResponse` to conform to how we handle other clients with go. So that no fields of the response need to be parsed after the client has returned the response.
// Client stores parameters common for internal-apis calls plus a logger.
type Client struct {
ServerAddress string
serverAddress string
AuthToken string
Logger *log.Logger
logger *log.Logger
tiger5226 commented 2019-07-14 03:56:07 +02:00 (Migrated from github.com)
Review

What is the purpose of making these private and adding methods? There is no special logic in the getter/setter to require it.

What is the purpose of making these private and adding methods? There is no special logic in the getter/setter to require it.
anbsky commented 2019-07-15 13:59:50 +02:00 (Migrated from github.com)
Review

Can't define properties on an interface.

Can't define properties on an interface.
}
// APIResponse reflects internal-apis JSON response format.
@ -38,16 +45,16 @@ const (
// NewClient returns a client instance for internal-apis. It requires authToken to be provided
// for authentication.
func NewClient(authToken string) Client {
return Client{
ServerAddress: defaultAPIHost,
func NewClient(authToken string) Caller {
return &Client{
serverAddress: defaultAPIHost,
AuthToken: authToken,
Logger: log.StandardLogger(),
logger: log.StandardLogger(),
}
}
func (c Client) getEndpointURL(object, method string) string {
return fmt.Sprintf("%s/%s/%s", c.ServerAddress, object, method)
return fmt.Sprintf("%s/%s/%s", c.GetServerAddress(), object, method)
}
func (c Client) prepareParams(params map[string]interface{}) (string, error) {
@ -62,9 +69,19 @@ func (c Client) prepareParams(params map[string]interface{}) (string, error) {
return form.Encode(), nil
}
// SetServerAddress overrides the default internal-apis server address.
func (c *Client) SetServerAddress(s string) {
c.serverAddress = s
}
// GetServerAddress returns currently defined internal-apis server address.
func (c Client) GetServerAddress() string {
tiger5226 commented 2019-07-14 03:57:27 +02:00 (Migrated from github.com)
Review

I specifically like this pattern. Does it allow for specially configured logging per type?

I specifically like this pattern. Does it allow for specially configured logging per type?
return c.serverAddress
}
func (c Client) doCall(url string, payload string) ([]byte, error) {
var body []byte
c.Logger.Debugf("sending payload: %s", payload)
c.logger.Debugf("sending payload: %s", payload)
req, err := http.NewRequest(http.MethodPost, url, bytes.NewBuffer([]byte(payload)))
if err != nil {
return body, err

View file

@ -55,10 +55,17 @@ func launchDummyServer() {
log.Fatal(s.ListenAndServe())
}
func TestClient_Set_GetServerAddress(t *testing.T) {
tiger5226 commented 2019-07-14 03:51:26 +02:00 (Migrated from github.com)
Review

should avoid underscores in go code method names.

should avoid underscores in go code method names.
c := NewClient("realToken")
assert.Equal(t, defaultAPIHost, c.GetServerAddress())
c.SetServerAddress("http://host.com/api")
assert.Equal(t, "http://host.com/api", c.GetServerAddress())
}
func TestUserMe(t *testing.T) {
go launchDummyServer()
c := NewClient("realToken")
c.ServerAddress = dummyServerURL
c.SetServerAddress(dummyServerURL)
r, err := c.UserMe()
assert.Nil(t, err)
assert.Equal(t, r["primary_email"], "andrey@lbry.com")