Formalizing support for site.ext.amp (prebid#711)

* Started adding some code to validate the amp ext, and some unit tests which dont pass yet.

* Fixed some bugs. Added more tests.

* Updated the docs.

* Fixed some tests... but not all.

* Fixed last remaining bug.

* Moved setAmpExt to the amp_auction file.

docs/endpoints/openrtb2/auction.md

@@ -80,7 +80,11 @@ The only exception here is the top-level `BidResponse`, because it's bidder-inde
 `ext.{anyBidderCode}` and `ext.bidder` extensions are defined by bidders.
 `ext.prebid` extensions are defined by Prebid Server.
 
-Exceptions are made for DigiTrust and GDPR, so that we define `ext` according to the official recommendations.
+Exceptions are made for extensions with "standard" recommendations:
+
+- `request.user.ext.digitrust` -- To support Digitrust support
+- `request.regs.ext.gdpr` and `request.user.ext.consent` -- To support GDPR
+- `request.site.ext.amp` -- To identify AMP as the request source
 
 #### Bid Adjustments
 
@@ -254,7 +258,7 @@ For example, a request may return this in `response.ext`
     ],
     "rubicon": [
       {
-        "code": 1, 
+        "code": 1,
         "message": "The request exceeded the timeout allocated"
       }
     ]

endpoints/openrtb2/amp_auction.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/buger/jsonparser"
 	"github.com/golang/glog"
 	"github.com/julienschmidt/httprouter"
 	"github.com/mxmCherry/openrtb"
@@ -280,6 +281,11 @@ func (deps *endpointDeps) loadRequestJSONForAmp(httpRequest *http.Request) (req
 		return
 	}
 
+	if req.App != nil {
+		errs = []error{errors.New("request.app must not exist in AMP stored requests.")}
+		return
+	}
+
 	// Force HTTPS as AMP requires it, but pubs can forget to set it.
 	if req.Imp[0].Secure == nil {
 		secure := int8(1)
@@ -294,6 +300,9 @@ func (deps *endpointDeps) loadRequestJSONForAmp(httpRequest *http.Request) (req
 }
 
 func (deps *endpointDeps) overrideWithParams(httpRequest *http.Request, req *openrtb.BidRequest) {
+	if req.Site == nil {
+		req.Site = &openrtb.Site{}
+	}
 	// Override the stored request sizes with AMP ones, if they exist.
 	if req.Imp[0].Banner != nil {
 		width := parseFormInt(httpRequest, "w", 0)
@@ -311,11 +320,7 @@ func (deps *endpointDeps) overrideWithParams(httpRequest *http.Request, req *ope
 
 	canonicalURL := httpRequest.FormValue("curl")
 	if canonicalURL != "" {
-		if req.Site == nil {
-			req.Site = &openrtb.Site{Page: canonicalURL}
-		} else {
-			req.Site.Page = canonicalURL
-		}
+		req.Site.Page = canonicalURL
 		// Fixes #683
 		if parsedURL, err := url.Parse(canonicalURL); err == nil {
 			domain := parsedURL.Host
@@ -326,6 +331,8 @@ func (deps *endpointDeps) overrideWithParams(httpRequest *http.Request, req *ope
 		}
 	}
 
+	setAmpExt(req.Site, "1")
+
 	slot := httpRequest.FormValue("slot")
 	if slot != "" {
 		req.Imp[0].TagID = slot
@@ -455,3 +462,15 @@ func defaultRequestExt(req *openrtb.BidRequest) (errs []error) {
 
 	return
 }
+
+func setAmpExt(site *openrtb.Site, value string) {
+	if len(site.Ext) > 0 {
+		if _, dataType, _, _ := jsonparser.Get(site.Ext, "amp"); dataType == jsonparser.NotExist {
+			if val, err := jsonparser.Set(site.Ext, []byte(value), "amp"); err == nil {
+				site.Ext = val
+			}
+		}
+	} else {
+		site.Ext = json.RawMessage(`{"amp":` + value + `}`)
+	}
+}

endpoints/openrtb2/amp_auction_test.go

@@ -30,12 +30,10 @@ func TestGoodAmpRequests(t *testing.T) {
 	goodRequests := map[string]json.RawMessage{
 		"1": json.RawMessage(validRequest(t, "aliased-buyeruids.json")),
 		"2": json.RawMessage(validRequest(t, "aliases.json")),
-		"3": json.RawMessage(validRequest(t, "app.json")),
 		"4": json.RawMessage(validRequest(t, "digitrust.json")),
 		"5": json.RawMessage(validRequest(t, "gdpr-no-consentstring.json")),
 		"6": json.RawMessage(validRequest(t, "gdpr.json")),
 		"7": json.RawMessage(validRequest(t, "site.json")),
-		"8": json.RawMessage(validRequest(t, "timeout.json")),
 		"9": json.RawMessage(validRequest(t, "user.json")),
 	}
 
@@ -99,6 +97,29 @@ func TestAMPPageInfo(t *testing.T) {
 	assert.Equal(t, "test.somepage.co.uk", exchange.lastRequest.Site.Domain)
 }
 
+func TestAMPSiteExt(t *testing.T) {
+	stored := map[string]json.RawMessage{
+		"1": json.RawMessage(validRequest(t, "site.json")),
+	}
+	theMetrics := pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList())
+	exchange := &mockAmpExchange{}
+	endpoint, _ := NewAmpEndpoint(exchange, newParamsValidator(t), &mockAmpStoredReqFetcher{stored}, &config.Configuration{MaxRequestSize: maxSize}, theMetrics, analyticsConf.NewPBSAnalytics(&config.Analytics{}))
+	request, err := http.NewRequest("GET", "/openrtb2/auction/amp?tag_id=1", nil)
+	if !assert.NoError(t, err) {
+		return
+	}
+	recorder := httptest.NewRecorder()
+	endpoint(recorder, request, nil)
+
+	if !assert.NotNil(t, exchange.lastRequest, "Endpoint responded with %d: %s", recorder.Code, recorder.Body.String()) {
+		return
+	}
+	if !assert.NotNil(t, exchange.lastRequest.Site) {
+		return
+	}
+	assert.JSONEq(t, `{"amp":1}`, string(exchange.lastRequest.Site.Ext))
+}
+
 // TestBadRequests makes sure we return 400's on bad requests.
 func TestAmpBadRequests(t *testing.T) {
 	files := fetchFiles(t, "sample-requests/invalid-whole")
@@ -126,7 +147,6 @@ func TestAmpBadRequests(t *testing.T) {
 // TestAmpDebug makes sure we get debug information back when requested
 func TestAmpDebug(t *testing.T) {
 	requests := map[string]json.RawMessage{
-		"1": json.RawMessage(validRequest(t, "app.json")),
 		"2": json.RawMessage(validRequest(t, "site.json")),
 	}
 

endpoints/openrtb2/auction.go

@@ -686,9 +686,19 @@ func (deps *endpointDeps) validateAliases(aliases map[string]string) error {
 }
 
 func (deps *endpointDeps) validateSite(site *openrtb.Site) error {
-	if site != nil && site.ID == "" && site.Page == "" {
+	if site == nil {
+		return nil
+	}
+
+	if site.ID == "" && site.Page == "" {
 		return errors.New("request.site should include at least one of request.site.id or request.site.page.")
 	}
+	if len(site.Ext) > 0 {
+		var s openrtb_ext.ExtSite
+		if err := json.Unmarshal(site.Ext, &s); err != nil {
+			return err
+		}
+	}
 
 	return nil
 }
@@ -792,6 +802,9 @@ func setSiteImplicitly(httpReq *http.Request, bidReq *openrtb.BidRequest) {
 			}
 		}
 	}
+	if bidReq.Site != nil {
+		setAmpExt(bidReq.Site, "0")
+	}
 }
 
 func setImpsImplicitly(httpReq *http.Request, imps []openrtb.Imp) {

endpoints/openrtb2/auction_test.go

@@ -491,6 +491,49 @@ func TestTimeoutParser(t *testing.T) {
 	}
 }
 
+func TestImplicitAMPNoExt(t *testing.T) {
+	httpReq, err := http.NewRequest("POST", "/openrtb2/auction", strings.NewReader(validRequest(t, "site.json")))
+	if !assert.NoError(t, err) {
+		return
+	}
+
+	bidReq := openrtb.BidRequest{
+		Site: &openrtb.Site{},
+	}
+	setSiteImplicitly(httpReq, &bidReq)
+	assert.JSONEq(t, `{"amp":0}`, string(bidReq.Site.Ext))
+}
+
+func TestImplicitAMPOtherExt(t *testing.T) {
+	httpReq, err := http.NewRequest("POST", "/openrtb2/auction", strings.NewReader(validRequest(t, "site.json")))
+	if !assert.NoError(t, err) {
+		return
+	}
+
+	bidReq := openrtb.BidRequest{
+		Site: &openrtb.Site{
+			Ext: json.RawMessage(`{"other":true}`),
+		},
+	}
+	setSiteImplicitly(httpReq, &bidReq)
+	assert.JSONEq(t, `{"amp":0,"other":true}`, string(bidReq.Site.Ext))
+}
+
+func TestExplicitAMP(t *testing.T) {
+	httpReq, err := http.NewRequest("POST", "/openrtb2/auction", strings.NewReader(validRequest(t, "site-amp.json")))
+	if !assert.NoError(t, err) {
+		return
+	}
+
+	bidReq := openrtb.BidRequest{
+		Site: &openrtb.Site{
+			Ext: json.RawMessage(`{"amp":1}`),
+		},
+	}
+	setSiteImplicitly(httpReq, &bidReq)
+	assert.JSONEq(t, `{"amp":1}`, string(bidReq.Site.Ext))
+}
+
 // TestContentType prevents #328
 func TestContentType(t *testing.T) {
 	endpoint, _ := NewEndpoint(

endpoints/openrtb2/sample-requests/invalid-whole/site-ext-amp.json

@@ -0,0 +1,47 @@
+{
+  "message": "Invalid request: request.site.ext.amp must be either 1, 0, or undefined\n",
+  "requestPayload": {
+    "id": "some-request-id",
+    "site": {
+      "page": "test.somepage.com",
+      "ext": {
+        "amp": 2
+      }
+    },
+    "imp": [
+      {
+        "id": "my-imp-id",
+        "banner": {
+          "format": [
+            {
+              "w": 300,
+              "h": 600
+            }
+          ]
+        },
+        "pmp": {
+          "deals": [
+            {
+              "id": "some-deal-id"
+            }
+          ]
+        },
+        "ext": {
+          "appnexus": {
+            "placementId": 10433394
+          }
+        }
+      }
+    ],
+    "ext": {
+      "prebid": {
+        "targeting": {
+          "pricegranularity": "low"
+        },
+        "cache": {
+          "bids": {}
+        }
+      }
+    }
+  }
+}

endpoints/openrtb2/sample-requests/valid-whole/supplementary/site-amp.json

@@ -0,0 +1,44 @@
+{
+  "id": "some-request-id",
+  "site": {
+    "page": "test.somepage.com",
+    "ext": {
+      "amp": 1
+    }
+  },
+  "imp": [
+    {
+      "id": "my-imp-id",
+      "banner": {
+        "format": [
+          {
+            "w": 300,
+            "h": 600
+          }
+        ]
+      },
+      "pmp": {
+        "deals": [
+          {
+            "id": "some-deal-id"
+          }
+        ]
+      },
+      "ext": {
+        "appnexus": {
+          "placementId": 10433394
+        }
+      }
+    }
+  ],
+  "ext": {
+    "prebid": {
+      "targeting": {
+        "pricegranularity": "low"
+      },
+      "cache": {
+        "bids": {}
+      }
+    }
+  }
+}

openrtb_ext/site.go

@@ -0,0 +1,34 @@
+package openrtb_ext
+
+import (
+	"errors"
+
+	"github.com/buger/jsonparser"
+)
+
+// ExtSite defines the contract for bidrequest.site.ext
+type ExtSite struct {
+	// AMP should be 1 if the request comes from an AMP page, and 0 if not.
+	AMP int8 `json:"amp"`
+}
+
+func (es *ExtSite) UnmarshalJSON(b []byte) error {
+	if len(b) == 0 {
+		return errors.New("request.site.ext must have some data in it")
+	}
+	if value, dataType, _, _ := jsonparser.Get(b, "amp"); dataType != jsonparser.NotExist && dataType != jsonparser.Number {
+		return errors.New(`request.site.ext.amp must be either 1, 0, or undefined`)
+	} else if len(value) != 1 {
+		return errors.New(`request.site.ext.amp must be either 1, 0, or undefined`)
+	} else {
+		switch value[0] {
+		case byte(48): // 0
+			es.AMP = 0
+		case byte(49): // 1
+			es.AMP = 1
+		default:
+			return errors.New(`request.site.ext.amp must be either 1, 0, or undefined`)
+		}
+	}
+	return nil
+}

openrtb_ext/site_test.go

@@ -0,0 +1,28 @@
+package openrtb_ext_test
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/prebid/prebid-server/openrtb_ext"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestInvalidSiteExt(t *testing.T) {
+	var s openrtb_ext.ExtSite
+	assert.EqualError(t, json.Unmarshal([]byte(`{"amp":-1}`), &s), "request.site.ext.amp must be either 1, 0, or undefined")
+	assert.EqualError(t, json.Unmarshal([]byte(`{"amp":2}`), &s), "request.site.ext.amp must be either 1, 0, or undefined")
+	assert.EqualError(t, json.Unmarshal([]byte(`{"amp":true}`), &s), "request.site.ext.amp must be either 1, 0, or undefined")
+	assert.EqualError(t, json.Unmarshal([]byte(`{"amp":null}`), &s), "request.site.ext.amp must be either 1, 0, or undefined")
+	assert.EqualError(t, json.Unmarshal([]byte(`{"amp":"1"}`), &s), "request.site.ext.amp must be either 1, 0, or undefined")
+}
+
+func TestValidSiteExt(t *testing.T) {
+	var s openrtb_ext.ExtSite
+	assert.NoError(t, json.Unmarshal([]byte(`{"amp":0}`), &s))
+	assert.EqualValues(t, 0, s.AMP)
+	assert.NoError(t, json.Unmarshal([]byte(`{"amp":1}`), &s))
+	assert.EqualValues(t, 1, s.AMP)
+	assert.NoError(t, json.Unmarshal([]byte(`{"amp":      1   }`), &s))
+	assert.EqualValues(t, 1, s.AMP)
+}