topdown/http: validate force_cache parameters

Earlier the validation of the force_cache parameters was performed
after creating the inter-query cache object. So if 'force_cache' was
set and 'force_cache_duration_seconds' was not provided, OPA in server mode
would correctly return an error but OPA running in repl mode would not
return an error and simply would fall back to using the intra-query cache.

This commit updates how the 'force_cache' parameters are validated so that
consistent behavior is seen in both repl and server modes.

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>

topdown/http.go

@@ -936,13 +936,13 @@ type httpRequestExecutor interface {
 // newHTTPRequestExecutor returns a new HTTP request executor that wraps either an inter-query or
 // intra-query cache implementation
 func newHTTPRequestExecutor(bctx BuiltinContext, key ast.Object) (httpRequestExecutor, error) {
-	useInterQueryCache, forceCache, err := useInterQueryCache(key)
+	useInterQueryCache, forceCacheParams, err := useInterQueryCache(key)
 	if err != nil {
 		return nil, handleHTTPSendErr(bctx, err)
 	}
 
 	if useInterQueryCache && bctx.InterQueryBuiltinCache != nil {
-		return newInterQueryCache(bctx, key, forceCache)
+		return newInterQueryCache(bctx, key, forceCacheParams)
 	}
 	return newIntraQueryCache(bctx, key)
 }
@@ -953,12 +953,11 @@ type interQueryCache struct {
 	httpReq          *http.Request
 	httpClient       *http.Client
 	forceJSONDecode  bool
-	forceCache       bool
 	forceCacheParams *forceCacheParams
 }
 
-func newInterQueryCache(bctx BuiltinContext, key ast.Object, forceCache bool) (*interQueryCache, error) {
-	return &interQueryCache{bctx: bctx, key: key, forceCache: forceCache}, nil
+func newInterQueryCache(bctx BuiltinContext, key ast.Object, forceCacheParams *forceCacheParams) (*interQueryCache, error) {
+	return &interQueryCache{bctx: bctx, key: key, forceCacheParams: forceCacheParams}, nil
 }
 
 // CheckCache checks the cache for the value of the key set on this object
@@ -975,13 +974,6 @@ func (c *interQueryCache) CheckCache() (ast.Value, error) {
 		return nil, handleHTTPSendErr(c.bctx, err)
 	}
 
-	if c.forceCache {
-		c.forceCacheParams, err = newForceCacheParams(c.key)
-		if err != nil {
-			return nil, handleHTTPSendErr(c.bctx, err)
-		}
-	}
-
 	resp, err := checkHTTPSendInterQueryCache(c.bctx, c.key, c.httpReq, c.httpClient, c.forceJSONDecode, c.forceCacheParams)
 
 	// fallback to the intra-query cache if response not found in the inter-query cache or inter-query cache look-up results
@@ -1051,18 +1043,23 @@ func (c *intraQueryCache) ExecuteHTTPRequest() (*http.Response, error) {
 	return executeHTTPRequest(httpReq, httpClient)
 }
 
-func useInterQueryCache(req ast.Object) (bool, bool, error) {
+func useInterQueryCache(req ast.Object) (bool, *forceCacheParams, error) {
 	value, err := getBoolValFromReqObj(req, ast.StringTerm("cache"))
 	if err != nil {
-		return false, false, err
+		return false, nil, err
 	}
 
 	valueForceCache, err := getBoolValFromReqObj(req, ast.StringTerm("force_cache"))
 	if err != nil {
-		return false, false, err
+		return false, nil, err
 	}
 
-	return value || valueForceCache, valueForceCache, nil
+	if valueForceCache {
+		forceCacheParams, err := newForceCacheParams(req)
+		return true, forceCacheParams, err
+	}
+
+	return value, nil, nil
 }
 
 type forceCacheParams struct {

topdown/http_test.go

@@ -1448,12 +1448,7 @@ func TestInterQueryCheckCacheError(t *testing.T) {
 	input := ast.MustParseTerm(`{"force_cache": true}`)
 	inputObj := input.Value.(ast.Object)
 
-	cache, err := newInterQueryCache(BuiltinContext{Context: context.Background()}, inputObj, true)
-	if err != nil {
-		t.Fatalf("unexpected error %v", err)
-	}
-
-	_, err = cache.CheckCache()
+	_, err := newHTTPRequestExecutor(BuiltinContext{Context: context.Background()}, inputObj)
 	if err == nil {
 		t.Fatal("expected error but got nil")
 	}