Exposing functions to independently apply actions pre-request and post-response #94
Conversation
jordan83
changed the title
| @@ -83,7 +86,7 @@ class TestContext extends RecaptchaContext { | |||
| log_messages: Array<[LogLevel, string[]]> = []; | |||
|
|
|||
| constructor(config: RecaptchaConfig) { | |||
| super(config); | |||
| super(JSON.parse(JSON.stringify(config))); | |||
There was a problem hiding this comment.
the config should already be a JS object? why is this necessary?
There was a problem hiding this comment.
The config needed to be deep-copied. It's global and there are tests that modify it so I had failures when making new tests. We can also have a function that creates a new literal. Do you have a preference?
There was a problem hiding this comment.
wild that there is no good way to do a deep copy.
I've been using the spread operator {...config} for stuff like this. though in cases where the object is nested (not here) it doesn't work.
Object.assign({}, config) would also work here (though (not relevant) strips methods and prototypes)
or consider just adding a comment so the motivation for the serialization is clear
There was a problem hiding this comment.
Adding a comment for now. We can always revisit.
src/index.test.ts
Outdated
| name: "test-policy", | ||
| description: "test-description", | ||
| path: "/teste2e", | ||
| // 'type' isn't a part of the interface, but is added for testing. |
There was a problem hiding this comment.
nit: I don't think the 'type' addition is required anymore; was previously helpful with Zod.
src/policy.ts
Outdated
| */ | ||
| export async function processRequest(context: RecaptchaContext, req: EdgeRequest): Promise<EdgeResponse> { | ||
| export async function getActions(context: RecaptchaContext, req: EdgeRequest): Promise<action.Action[]> { |
There was a problem hiding this comment.
'getActions' sounds like a lightweight operation, where this can actually call 2 RPCs (ListFirewallPolicies (cachable) and CreateAssessment).
consider something like:
FetchActions, RetrieveActions, AcquireActions, ProduceActions, EvaluatePolicyProduceActions....
There was a problem hiding this comment.
Yeah, good idea. Will do.
- getActions => fetchActions - removing type field from actions in tests - deep copy is clear when copying the config in tests
No description provided.