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

Add http api for fetching accounts #1545

Merged
merged 7 commits into from
Oct 22, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -652,8 +652,7 @@ func TestValidateAccountsConfigRestrictions(t *testing.T) {
cfg.Accounts.Postgres.ConnectionInfo.Database = "accounts"

errs := cfg.validate()
assert.Len(t, errs, 2)
assert.Contains(t, errs, errors.New("accounts.http: retrieving accounts via http not available, use accounts.files"))
assert.Len(t, errs, 1)
assert.Contains(t, errs, errors.New("accounts.postgres: retrieving accounts via postgres not available, use accounts.files"))
}

Expand Down
3 changes: 0 additions & 3 deletions config/stored_requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,6 @@ func resolvedStoredRequestsConfig(cfg *Configuration) {
}

func (cfg *StoredRequests) validate(errs configErrors) configErrors {
if cfg.DataType() == AccountDataType && cfg.HTTP.Endpoint != "" {
errs = append(errs, fmt.Errorf("%s.http: retrieving accounts via http not available, use accounts.files", cfg.Section()))
}
if cfg.DataType() == AccountDataType && cfg.Postgres.ConnectionInfo.Database != "" {
errs = append(errs, fmt.Errorf("%s.postgres: retrieving accounts via postgres not available, use accounts.files", cfg.Section()))
} else {
Expand Down
91 changes: 86 additions & 5 deletions stored_requests/backends/http_fetcher/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io/ioutil"
"net/http"
"net/url"
"strings"

"github.com/prebid/prebid-server/stored_requests"
Expand All @@ -19,9 +20,13 @@ import (
//
// This file expects the endpoint to satisfy the following API:
//
// Stored requests
// GET {endpoint}?request-ids=["req1","req2"]&imp-ids=["imp1","imp2","imp3"]
//
// This endpoint should return a payload like:
// Accounts
// GET {endpoint}?account-ids=["acc1","acc2"]
//
// The above endpoints should return a payload like:
//
// {
// "requests": {
Expand All @@ -34,20 +39,32 @@ import (
// "imp3": null // If imp3 is not found
// }
// }
// or
// {
// "accounts": {
// "acc1": { ... config data for acc1 ... },
// "acc2": { ... config data for acc2 ... },
// },
// }
//
//
func NewFetcher(client *http.Client, endpoint string) *HttpFetcher {
// Do some work up-front to figure out if the (configurable) endpoint has a query string or not.
// When we build requests, we'll either want to add `?request-ids=...&imp-ids=...` _or_
// `&request-ids=...&imp-ids=...`, depending.
// `&request-ids=...&imp-ids=...`.

urlPrefix := endpoint
if strings.Contains(endpoint, "?") {
urlPrefix = urlPrefix + "&"
} else {
urlPrefix = urlPrefix + "?"
}

glog.Info("Making http_fetcher which calls GET " + urlPrefix + "request-ids=%REQUEST_ID_LIST%&imp-ids=%IMP_ID_LIST%")
if url, err := url.Parse(endpoint); err != nil {
glog.Warningf(`Invalid endpoint "%s": %v`, endpoint, err)
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
} else {
glog.Infof("Making http_fetcher for endpoint %v", url)
}

return &HttpFetcher{
client: client,
Expand Down Expand Up @@ -81,8 +98,68 @@ func (fetcher *HttpFetcher) FetchRequests(ctx context.Context, requestIDs []stri
return
}

func (fetcher *HttpFetcher) FetchAccount(ctx context.Context, accountID string) (json.RawMessage, []error) {
return nil, []error{stored_requests.NotFoundError{accountID, "Account"}}
// FetchAccounts retrieves account configurations
//
// Request format is similar to the one for requests:
// GET {endpoint}?account-ids=["account1","account2",...]
//
// The endpoint is expected to respond with a JSON map with accountID -> json.RawMessage
// {
// "account1": { ... account json ... }
// }
// The JSON contents of account config is returned as-is (NOT validated)
func (fetcher *HttpFetcher) FetchAccounts(ctx context.Context, accountIDs []string) (accountData map[string]json.RawMessage, errs []error) {
if len(accountIDs) == 0 {
return nil, nil
}
httpReq, err := http.NewRequestWithContext(ctx, "GET", fetcher.Endpoint+"account-ids=[\""+strings.Join(accountIDs, "\",\"")+"\"]", nil)
if err != nil {
return nil, []error{
fmt.Errorf(`Error fetching accounts "%v" via http: %v`, accountIDs, err),
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
}
}
httpResp, err := ctxhttp.Do(ctx, fetcher.client, httpReq)
if err != nil {
return nil, []error{
fmt.Errorf(`Error fetching accounts %v via http: %v`, accountIDs, err),
}
}
defer httpResp.Body.Close()
respBytes, err := ioutil.ReadAll(httpResp.Body)
if err != nil {
return nil, []error{
fmt.Errorf(`Error fetching accounts %v via http: could not read response`, accountIDs),
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
}
}
if httpResp.StatusCode != http.StatusOK {
return nil, []error{
fmt.Errorf(`Error fetching accounts %v via http: unexpected response status %d`, accountIDs, httpResp.StatusCode),
}
}
var responseData accountsResponseContract
if err = json.Unmarshal(respBytes, &responseData); err != nil {
return nil, []error{
fmt.Errorf(`Error fetching accounts %v via http: malformed response`, accountIDs),
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
}
}
errs = convertNullsToErrs(responseData.Accounts, "Account", errs)
return responseData.Accounts, errs
}

// FetchAccount fetchers a single accountID and returns its corresponding json
func (fetcher *HttpFetcher) FetchAccount(ctx context.Context, accountID string) (accountJSON json.RawMessage, errs []error) {
accountData, errs := fetcher.FetchAccounts(ctx, []string{accountID})
if len(errs) > 0 {
return nil, errs
}
accountJSON, ok := accountData[accountID]
if !ok {
return nil, []error{stored_requests.NotFoundError{
ID: accountID,
DataType: "Account",
}}
}
return accountJSON, []error{}
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
}

func (fetcher *HttpFetcher) FetchCategories(ctx context.Context, primaryAdServer, publisherId, iabCategory string) (string, error) {
Expand Down Expand Up @@ -190,3 +267,7 @@ type responseContract struct {
Requests map[string]json.RawMessage `json:"requests"`
Imps map[string]json.RawMessage `json:"imps"`
}

type accountsResponseContract struct {
Accounts map[string]json.RawMessage `json:"accounts"`
}
82 changes: 71 additions & 11 deletions stored_requests/backends/http_fetcher/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"net/http/httptest"
"strings"
"testing"

"github.com/stretchr/testify/assert"
)

func TestSingleReq(t *testing.T) {
Expand Down Expand Up @@ -61,14 +63,39 @@ func TestReqsAndImps(t *testing.T) {
assertErrLength(t, errs, 0)
}

func TestMissingValues(t *testing.T) {
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
fetcher, close := newEmptyFetcher(t, []string{"req-1", "req-2"}, []string{"imp-1"})
func TestFetchAccounts(t *testing.T) {
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
fetcher, close := newTestAccountFetcher(t, []string{"acc-1", "acc-2"})
defer close()

reqData, impData, errs := fetcher.FetchRequests(context.Background(), []string{"req-1", "req-2"}, []string{"imp-1"})
assertMapKeys(t, reqData)
assertMapKeys(t, impData)
assertErrLength(t, errs, 3)
accData, errs := fetcher.FetchAccounts(context.Background(), []string{"acc-1", "acc-2"})
assertMapKeys(t, accData, "acc-1", "acc-2")
assertErrLength(t, errs, 0)
}

func TestFetchAccountsNoData(t *testing.T) {
fetcher, close := newFetcherBrokenBackend()
defer close()
accData, errs := fetcher.FetchAccounts(context.Background(), []string{"req-1"})
assertMapKeys(t, accData)
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
assertErrLength(t, errs, 1)
}

func TestFetchAccount(t *testing.T) {
fetcher, close := newTestAccountFetcher(t, []string{"acc-1"})
defer close()

account, errs := fetcher.FetchAccount(context.Background(), "acc-1")
assert.Empty(t, errs, "Unexpected error fetching existing account")
assert.JSONEq(t, `"acc-1"`, string(account))
laurb9 marked this conversation as resolved.
Show resolved Hide resolved
}

laurb9 marked this conversation as resolved.
Show resolved Hide resolved
func TestFetchAccountNoData(t *testing.T) {
fetcher, close := newFetcherBrokenBackend()
defer close()

unknownAccount, errs := fetcher.FetchAccount(context.Background(), "unknown-acc")
assert.NotEmpty(t, errs, "Retrieving unknown account should have returned an error")
assert.Nil(t, unknownAccount)
}

func TestErrResponse(t *testing.T) {
Expand Down Expand Up @@ -139,12 +166,12 @@ func newTestFetcher(t *testing.T, expectReqIDs []string, expectImpIDs []string)
func newHandler(t *testing.T, expectReqIDs []string, expectImpIDs []string, jsonifier func(string) json.RawMessage) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
query := r.URL.Query()
assertMatches(t, query.Get("request-ids"), expectReqIDs)
assertMatches(t, query.Get("imp-ids"), expectImpIDs)

gotReqIDs := richSplit(query.Get("request-ids"))
gotImpIDs := richSplit(query.Get("imp-ids"))

assertMatches(t, gotReqIDs, expectReqIDs)
assertMatches(t, gotImpIDs, expectImpIDs)

reqIDResponse := make(map[string]json.RawMessage, len(gotReqIDs))
impIDResponse := make(map[string]json.RawMessage, len(gotImpIDs))

Expand Down Expand Up @@ -174,10 +201,43 @@ func newHandler(t *testing.T, expectReqIDs []string, expectImpIDs []string, json
}
}

func assertMatches(t *testing.T, query string, expected []string) {
func newTestAccountFetcher(t *testing.T, expectAccIDs []string) (fetcher *HttpFetcher, closer func()) {
handler := newAccountHandler(t, expectAccIDs, jsonifyID)
server := httptest.NewServer(http.HandlerFunc(handler))
return NewFetcher(server.Client(), server.URL), server.Close
}

func newAccountHandler(t *testing.T, expectAccIDs []string, jsonifier func(string) json.RawMessage) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
query := r.URL.Query()
gotAccIDs := richSplit(query.Get("account-ids"))

assertMatches(t, gotAccIDs, expectAccIDs)

accIDResponse := make(map[string]json.RawMessage, len(gotAccIDs))

for _, accID := range gotAccIDs {
if accID != "" {
accIDResponse[accID] = jsonifier(accID)
}
}

respObj := accountsResponseContract{
Accounts: accIDResponse,
}

if respBytes, err := json.Marshal(respObj); err != nil {
t.Errorf("failed to marshal responseContract in test: %v", err)
w.WriteHeader(http.StatusInternalServerError)
} else {
w.Write(respBytes)
}
}
}

func assertMatches(t *testing.T, queryVals []string, expected []string) {
t.Helper()

queryVals := richSplit(query)
if len(queryVals) == 1 && queryVals[0] == "" {
if len(expected) != 0 {
t.Errorf("Expected no query vals, but got %v", queryVals)
Expand Down