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

verify: refactor Go/JSON API #28

Open
admtnnr opened this issue Aug 24, 2015 · 0 comments
Open

verify: refactor Go/JSON API #28

admtnnr opened this issue Aug 24, 2015 · 0 comments
Assignees
Milestone

Comments

@admtnnr
Copy link
Contributor

admtnnr commented Aug 24, 2015

There is an unnecessary amount of duplication between verifications and filters. Additionally, the verifiers have inconsistent behavior as to whether the verification requirement is 0, 1, or n times. It would be nice to create an API that handles the verification requirement in one place while allowing filters to be used or verification requirements.

Example API

For example, using the basic verification framework together with a url.Filter to create a verification for "all requests must be HTTPS":

v := verify.NewVerification("HTTPS enforcement")
c := v.VerifyAll()
// c := v.VerifyAny()
// c := v.VerifyCount(10)
// c := v.VerifyNone()

f := martianurl.NewFilter(&url.URL{Scheme: "https"})
f.SetRequestModifier(c)

v.SetRequestModifier(f)

Or in the JSON API:

{
  "verify.Verification": {
    "name": "HTTPS enforcement",
    "modifier": {
      "url.Filter": {
        "scheme": "https",
        "modifier": {
          "verify.All": { }
        }
      }
    }
  }
}

Implementation Details

Conditions need to be tied to their parent verify.Verification.

Go API

In the Go API this is simple enough since we can just have the Verification keep track of it's child conditions using the constructors available on Verification (VerifyAll, VerifyNone, etc.).

This will allow us to ask the verification container to generate errors without requiring complete traversal of the modifier tree. Additionally, this will allow us to remove all the existing methods on filters and groups that currently deal with passing along verification information through the stack (VerifyRequests, VerifyResponses, ResetRequestVerifications, and ResetResponseVerifications).

JSON API

The parse API is trickier. The JSON API currently does not provide any context during the parse and thus has no way to tie a modifier with the key verify.All to the outer verify.Verification.

The only solution I currently have to this is to provide a *parse.Context that can carry this information through the parse phase. It should itself be similar to net/context's Context such that we can push and pop the current verify.Verification off as needed since verify.Verifications may be nested underneath of each other, though I currently don't have a good explanation for when that would even make sense. Maybe as a general context API for others to use in case they need to store information that is nestable.

@admtnnr admtnnr added this to the v2.1.0 milestone Aug 24, 2015
@admtnnr admtnnr changed the title verify: rework Go API verify: refactor Go/JSON API Aug 24, 2015
@admtnnr admtnnr self-assigned this Dec 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant