diff --git a/.changeset/cold-bikes-sit.md b/.changeset/cold-bikes-sit.md new file mode 100644 index 00000000..6ace567f --- /dev/null +++ b/.changeset/cold-bikes-sit.md @@ -0,0 +1,22 @@ +--- +"@styra/opa-react": minor +--- + +Support batching requests sent to the backend (optional) + +When used with [Enterprise OPA's Batch API](https://docs.styra.com/enterprise-opa/reference/api-reference/batch-api), this mode allows for sending much +fewer requests to the backend. It's enabled by setting `batch={true}` on ``. + +Note that the Batch API has no notion of "default query", so it's not possible +to use batching without having either `defaultPath` (``) or +`path` (`useAuthz()`, ``) set. + +Please note that `fromResult` is exempt from the cache key, so multiple requests +with the same path and input, but different `fromResult` settings will lead to +unforeseen results. +This is on par with the regular (non-batching) caching, and we'll revisit this +if it becomes a problem for users. Please create an issue on Github if it is +problematic for you. + +Furthermore, batching queries are not wired up with `AbortController` like the +non-batching equivalents are. diff --git a/.github/workflows/pull-request.yaml b/.github/workflows/pull-request.yaml index dac7d795..7201a339 100644 --- a/.github/workflows/pull-request.yaml +++ b/.github/workflows/pull-request.yaml @@ -70,7 +70,7 @@ jobs: run: npx -w ${{ matrix.pkg }} attw --pack --ignore-rules no-resolution if: matrix.pkg == 'packages/opa' - name: jsr publish dry-run - run: npx -w ${{ matrix.pkg }} jsr publish --dry-run + run: npx -w ${{ matrix.pkg }} jsr publish --dry-run --allow-dirty if: matrix.pkg == 'packages/opa' success: diff --git a/package-lock.json b/package-lock.json index 2a3f45bc..7cbefd6b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2456,6 +2456,21 @@ "integrity": "sha512-/LLMVyas0ljjAtoYiPqYiL8VWXzUUdThrmU5+n20DZv+a+ClRoevUzw5JxU+Ieh5/c87ytoTBV9G1FiKfNJdmg==", "dev": true }, + "node_modules/@yornaath/batshit": { + "version": "0.10.1", + "resolved": "https://registry.npmjs.org/@yornaath/batshit/-/batshit-0.10.1.tgz", + "integrity": "sha512-WGZ1WNoiVN6CLf28O73+6SCf+2lUn4U7TLGM9f4zOad0pn9mdoXIq8cwu3Kpf7N2OTYgWGK4eQPTflwFlduDGA==", + "license": "MIT", + "dependencies": { + "@yornaath/batshit-devtools": "^1.7.1" + } + }, + "node_modules/@yornaath/batshit-devtools": { + "version": "1.7.1", + "resolved": "https://registry.npmjs.org/@yornaath/batshit-devtools/-/batshit-devtools-1.7.1.tgz", + "integrity": "sha512-AyttV1Njj5ug+XqEWY1smV45dTWMlWKtj1B8jcFYgBKUFyUlF/qEhD+iP1E5UaRYW6hQRYD9T2WNDwFTrOMWzQ==", + "license": "MIT" + }, "node_modules/acorn": { "version": "8.12.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.12.0.tgz", @@ -9437,6 +9452,7 @@ "dependencies": { "@styra/opa": ">=1.1.3", "@tanstack/react-query": "^5.50.1", + "@yornaath/batshit": "^0.10.1", "lodash.merge": "^4.6.2", "zod": "^3.23.8" }, diff --git a/packages/opa-react/package.json b/packages/opa-react/package.json index 61fc3c8b..a3c6b477 100644 --- a/packages/opa-react/package.json +++ b/packages/opa-react/package.json @@ -39,6 +39,7 @@ "dependencies": { "@styra/opa": ">=1.1.3", "@tanstack/react-query": "^5.50.1", + "@yornaath/batshit": "^0.10.1", "lodash.merge": "^4.6.2", "zod": "^3.23.8" }, diff --git a/packages/opa-react/src/authz-provider.tsx b/packages/opa-react/src/authz-provider.tsx index a5d1b6c3..e0851470 100644 --- a/packages/opa-react/src/authz-provider.tsx +++ b/packages/opa-react/src/authz-provider.tsx @@ -14,6 +14,48 @@ import { type BatchRequestOptions, } from "@styra/opa"; import { type ServerError } from "@styra/opa/sdk/models/components"; +import { create, windowScheduler } from "@yornaath/batshit"; + +type EvalQuery = { + path: string | undefined; + input: Input | undefined; + fromResult: ((_?: Result) => boolean) | undefined; +}; + +function key({ path, input }: EvalQuery): string { + return stringify({ path, input }); // Note: omit fromResult +} + +const evals = (sdk: OPAClient) => + create({ + fetcher: async (evals: EvalQuery[]) => { + const evs = evals.map((x) => ({ ...x, k: key(x) })); + const groups = Object.groupBy(evs, ({ path }) => path); // group by path + return Promise.all( + Object.entries(groups).map(([path, inputs]) => { + const inps: Record = {}; + const fromRs: Record boolean> = {}; + inputs.forEach(({ k, input, fromResult }) => { + inps[k] = input; + fromRs[k] = fromResult; + }); + return sdk + .evaluateBatch(path, inps, { rejectMixed: true }) + .then((res) => + Object.fromEntries( + Object.entries(res).map(([k, res]) => [ + k, + fromRs[k] ? fromRs[k](res) : res, + ]), + ), + ); + }), + ).then((all: object[]) => all.reduce((acc, n) => ({ ...acc, ...n }), {})); // combine result arrays of objects + }, + resolver: (results, query) => results[key(query)] ?? null, + scheduler: windowScheduler(10), + name: "@styra/opa-react", + }); /** Abstracts the methods that are used from `OPAClient` of `@styra/opa`. */ export interface OPAClient { @@ -53,9 +95,6 @@ export type AuthzProviderContext = { /** The `@tanstack/react-query` client that's used for scheduling policy evaluation requests. */ queryClient: QueryClient; - - /** Whether or not policy evaluations should retry on transient failures. `false` means never; `true` means infinite retry; any number N means N retries. Defaults to 3. */ - retry: boolean | number; }; // Reference: https://reacttraining.com/blog/react-context-with-typescript @@ -63,9 +102,14 @@ export const AuthzContext = createContext( undefined, ); -export type AuthzProviderProps = PropsWithChildren< - Omit ->; +export interface AuthzProviderProps + extends PropsWithChildren> { + /** Whether or not policy evaluations should retry on transient failures. `false` means never; `true` means infinite retry; any number N means N retries. Defaults to 3. */ + retry?: boolean | number; + + /** Batch policy evaluation queries when possible, and supported by the backend. Defaults to `false`. */ + batch?: boolean; +} /** * Configures the authorization SDK, with default path/input of applicable. @@ -86,25 +130,43 @@ export default function AuthzProvider({ defaultPath, defaultInput, defaultFromResult, - retry = 3, + retry = 0, // Debugging + batch = false, }: AuthzProviderProps) { + const batcher = useMemo( + () => batch && opaClient && evals(opaClient), + [opaClient, batch], + ); const defaultQueryFn = useCallback( - async ({ queryKey, meta = {}, signal }: QueryFunctionContext) => { - if (!opaClient) return; - + async ({ + queryKey, + meta = {}, + signal, + }: QueryFunctionContext): Promise => { const [path, input] = queryKey as [string, Input]; - const fromResult = meta["fromResult"] as (_?: Result) => boolean; - return path - ? opaClient.evaluate(path, input, { - fromResult, - fetchOptions: { signal }, - }) - : opaClient.evaluateDefault(input, { - fromResult, - fetchOptions: { signal }, - }); + const fromResult = meta["fromResult"] as + | ((_?: Result) => boolean) + | undefined; + + if (!batcher) { + // use the default, unbatched queries backed by react-query + return path + ? opaClient.evaluate(path, input, { + fromResult, + fetchOptions: { signal }, + }) + : opaClient.evaluateDefault(input, { + fromResult, + fetchOptions: { signal }, + }); + } + + if (!path) + throw new Error("batch requests need to have a defined query path"); + + return batcher.fetch({ path, input, fromResult }); }, - [opaClient], + [batcher, batch], ); const queryClient = useMemo( () => @@ -116,7 +178,7 @@ export default function AuthzProvider({ }, }, }), - [defaultQueryFn], + [defaultQueryFn, retry], ); const context = useMemo( @@ -126,16 +188,8 @@ export default function AuthzProvider({ defaultInput, defaultFromResult, queryClient: queryClient!, - retry, }), - [ - opaClient, - defaultPath, - defaultInput, - defaultFromResult, - queryClient, - retry, - ], + [opaClient, defaultPath, defaultInput, defaultFromResult, queryClient], ); if (!queryClient) return null; @@ -144,3 +198,53 @@ export default function AuthzProvider({ {children} ); } + +// Taken from fast-json-stable-hash, MIT-licensed: +// https://github.com/zkldi/fast-json-stable-hash/blob/31b3081e942c1ce491f9698fd0bf527847093036/index.js +// That module was tricky to import because it's using `crypto` for hashing. +// We only need a stable string. +function stringify(obj: any) { + const type = typeof obj; + if (obj === undefined) return "_"; + + if (type === "string") { + return JSON.stringify(obj); + } else if (Array.isArray(obj)) { + let str = "["; + + let al = obj.length - 1; + + for (let i = 0; i < obj.length; i++) { + str += stringify(obj[i]); + + if (i !== al) { + str += ","; + } + } + + return `${str}]`; + } else if (type === "object" && obj !== null) { + let str = "{"; + let keys = Object.keys(obj).sort(); + + let kl = keys.length - 1; + + for (let i = 0; i < keys.length; i++) { + let key = keys[i]; + str += `${JSON.stringify(key)}:${stringify(obj[key])}`; + + if (i !== kl) { + str += ","; + } + } + + return `${str}}`; + } else if (type === "number" || type === "boolean" || obj === null) { + // bool, num, null have correct auto-coercions + return `${obj}`; + } else { + throw new TypeError( + `Invalid JSON type of ${type}, value ${obj}. Can only hash JSON objects.`, + ); + } +} diff --git a/packages/opa-react/src/package.json b/packages/opa-react/src/package.json new file mode 100644 index 00000000..3dbc1ca5 --- /dev/null +++ b/packages/opa-react/src/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/packages/opa-react/src/use-authz.ts b/packages/opa-react/src/use-authz.ts index 19c9e507..eeb33e89 100644 --- a/packages/opa-react/src/use-authz.ts +++ b/packages/opa-react/src/use-authz.ts @@ -26,7 +26,13 @@ export default function useAuthz( if (context === undefined) { throw Error("Authz/useAuthz can only be used inside an AuthzProvider"); } - const { defaultPath, defaultInput, defaultFromResult, queryClient } = context; + const { + defaultPath, + defaultInput, + defaultFromResult, + queryClient, + opaClient, + } = context; const p = path ?? defaultPath; const i = mergeInput(input, defaultInput); const fromR = fromResult ?? defaultFromResult; @@ -40,6 +46,7 @@ export default function useAuthz( { queryKey: [p, i], meta: { fromResult: fromR }, + enabled: !!opaClient, }, queryClient, ); diff --git a/packages/opa-react/tests/use-authz.test.tsx b/packages/opa-react/tests/use-authz.test.tsx index 7714961d..5af9ee91 100644 --- a/packages/opa-react/tests/use-authz.test.tsx +++ b/packages/opa-react/tests/use-authz.test.tsx @@ -16,6 +16,7 @@ describe("useAuthz Hook", () => { defaultPath, defaultInput, defaultFromResult, + batch = false, }: Omit = {}) => { return { wrapper: ({ children }) => ( @@ -25,6 +26,7 @@ describe("useAuthz Hook", () => { defaultInput={defaultInput} defaultFromResult={defaultFromResult} retry={false} + batch={batch} > {children} @@ -263,5 +265,223 @@ describe("useAuthz Hook", () => { expect.anything(), ); }); + + it("caches requests with the same input/path (disregarding fromResult)", async () => { + // NOTE(sr): We're mocking the call to evaluate which handles the `fromResult` + // application. So, it never really is called. However, we check that it was + // passed as expected in the assertions below. + const evaluateSpy = vi.spyOn(opa, "evaluate").mockResolvedValue(false); + + const fromResult1 = (res?: Result) => (res as any).foo; + const fromResult2 = (res?: Result) => (res as any).bar; + + const { result } = renderHook(() => { + return { + first: useAuthz("path/allow", "foo", fromResult1), + second: useAuthz( + "path/allow", + "foo", + fromResult2, // Not part of the cache key! + ), + }; + }, wrapper()); + + await waitFor(() => + Promise.all([ + expect(result.current.first).toMatchObject({ + isLoading: false, + error: undefined, + result: false, + }), + expect(result.current.second).toMatchObject({ + isLoading: false, + error: undefined, + result: false, + }), + ]), + ); + + expect(evaluateSpy).toHaveBeenCalledOnce(); + expect(evaluateSpy).toHaveBeenCalledWith("path/allow", "foo", { + fetchOptions: expect.anything(), + fromResult: fromResult1, + }); + }); + }); + + describe("with batching", () => { + const batch = true; + + it("works without input, without fromResult", async () => { + const hash = '{"input":_,"path":"path/allow"}'; + const evaluateSpy = vi + .spyOn(opa, "evaluateBatch") + .mockResolvedValue({ [hash]: false }); + + const { result } = renderHook( + () => useAuthz("path/allow"), + wrapper({ batch }), + ); + await waitFor(() => + expect(result.current).toMatchObject({ + isLoading: false, + error: undefined, + result: false, + }), + ); + expect(evaluateSpy).toHaveBeenCalledWith( + "path/allow", + { [hash]: undefined }, + { rejectMixed: true }, + ); + }); + + it("works without input, with fromResult", async () => { + const hash = '{"input":_,"path":"path/allow"}'; + const evaluateSpy = vi + .spyOn(opa, "evaluateBatch") + .mockResolvedValue({ [hash]: { foo: false } }); + + const { result } = renderHook( + () => useAuthz("path/allow", undefined, (x) => (x as any).foo), + wrapper({ batch }), + ); + await waitFor(() => + expect(result.current).toMatchObject({ + isLoading: false, + error: undefined, + result: false, + }), + ); + expect(evaluateSpy).toHaveBeenCalledWith( + "path/allow", + { [hash]: undefined }, + { rejectMixed: true }, + ); + }); + + it("rejects evals without path", async () => { + const hash = '{"input":_,"path":"path/allow"}'; + const evaluateSpy = vi.spyOn(opa, "evaluateBatch"); + + const { result } = renderHook( + () => useAuthz(undefined, "some input"), + wrapper({ batch }), + ); + await waitFor(() => + expect(result.current).toMatchObject({ + isLoading: false, + error: new Error("batch requests need to have a defined query path"), + result: undefined, + }), + ); + expect(evaluateSpy).not.toHaveBeenCalled(); + }); + + it("works with input, with fromResult", async () => { + const hash = '{"input":"foo","path":"path/allow"}'; + const evaluateSpy = vi + .spyOn(opa, "evaluateBatch") + .mockResolvedValue({ [hash]: { foo: false } }); + + const { result } = renderHook( + () => useAuthz("path/allow", "foo", (x) => (x as any).foo), + wrapper({ batch }), + ); + await waitFor(() => + expect(result.current).toMatchObject({ + isLoading: false, + error: undefined, + result: false, + }), + ); + expect(evaluateSpy).toHaveBeenCalledWith( + "path/allow", + { [hash]: "foo" }, + { rejectMixed: true }, + ); + }); + + it("batches multiple requests with different inputs", async () => { + const hash1 = '{"input":"foo","path":"path/allow"}'; + const hash2 = '{"input":"bar","path":"path/allow"}'; + const evaluateSpy = vi + .spyOn(opa, "evaluateBatch") + .mockResolvedValue({ [hash1]: false, [hash2]: true }); + + const { result } = renderHook(() => { + return { + first: useAuthz("path/allow", "foo"), + second: useAuthz("path/allow", "bar"), + }; + }, wrapper({ batch })); + + await waitFor(() => + Promise.all([ + expect(result.current.first).toMatchObject({ + isLoading: false, + error: undefined, + result: false, + }), + expect(result.current.second).toMatchObject({ + isLoading: false, + error: undefined, + result: true, + }), + ]), + ); + + expect(evaluateSpy).toHaveBeenCalledWith( + "path/allow", + { [hash1]: "foo", [hash2]: "bar" }, + { rejectMixed: true }, + ); + }); + + it("coalesces multiple requests with the same path input (disregarding fromResult)", async () => { + const hash = '{"input":"foo","path":"path/allow"}'; + // NOTE(sr): Unlike the non-batching case, we're handling the application of `fromResult` + // in the code of opa-react. So the functions that are passed are evaluated on the mocked + // result returned here. + const evaluateSpy = vi + .spyOn(opa, "evaluateBatch") + .mockResolvedValue({ [hash]: { foo: false } }); + + const { result } = renderHook(() => { + return { + first: useAuthz( + "path/allow", + "foo", + (res?: Result) => (res as any).foo, + ), + second: useAuthz( + "path/allow", + "foo", + (res?: Result) => (res as any).bar, + ), + }; + }, wrapper({ batch })); + + await waitFor(() => + Promise.all([ + expect(result.current.first).toMatchObject({ + isLoading: false, + error: undefined, + result: false, + }), + expect(result.current.second).toMatchObject({ + isLoading: false, + error: undefined, + result: false, + }), + ]), + ); + + expect(evaluateSpy).toHaveBeenCalledWith( + "path/allow", + { [hash]: "foo" }, + { rejectMixed: true }, + ); + }); }); }); diff --git a/packages/opa-react/tsconfig.json b/packages/opa-react/tsconfig.json index 59bcf698..609ce827 100644 --- a/packages/opa-react/tsconfig.json +++ b/packages/opa-react/tsconfig.json @@ -1,5 +1,6 @@ { "compilerOptions": { + "lib": ["ESNext.Object", "DOM"], "allowSyntheticDefaultImports": true, "jsx": "react-jsx", "declaration": true,