-
Notifications
You must be signed in to change notification settings - Fork 26
Use standard io Writer and Reader where possible #101
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
Changes from all commits
fb9f708
119ae89
1c8cd96
971afd9
9572c76
7ca921b
4160a94
e828582
9bc0483
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| package parser_test | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "io" | ||
| "testing" | ||
|
|
||
| "github.com/cedar-policy/cedar-go/internal/parser" | ||
| "github.com/cedar-policy/cedar-go/internal/testutil" | ||
| "github.com/cedar-policy/cedar-go/x/exp/ast" | ||
| ) | ||
|
|
||
| func TestEncoder(t *testing.T) { | ||
| var buf bytes.Buffer | ||
|
|
||
| encoder := parser.NewEncoder(&buf) | ||
|
|
||
| policy := ast.Permit(). | ||
| PrincipalEq(johnny). | ||
| ActionEq(sow). | ||
| ResourceEq(apple). | ||
| When(ast.Boolean(true)). | ||
| Unless(ast.Boolean(false)) | ||
|
|
||
| err := encoder.Encode((*parser.Policy)(policy)) | ||
| testutil.OK(t, err) | ||
|
|
||
| const expected = `permit ( | ||
| principal == User::"johnny", | ||
| action == Action::"sow", | ||
| resource == Crop::"apple" | ||
| ) | ||
| when { true } | ||
| unless { false }; | ||
| ` | ||
|
|
||
| testutil.Equals(t, buf.String(), expected) | ||
| } | ||
|
|
||
| func TestEncoderError(t *testing.T) { | ||
| _, w := io.Pipe() | ||
| _ = w.Close() | ||
|
|
||
| encoder := parser.NewEncoder(w) | ||
|
|
||
| policy := ast.Permit(). | ||
| PrincipalEq(johnny). | ||
| ActionEq(sow). | ||
| ResourceEq(apple). | ||
| When(ast.Boolean(true)). | ||
| Unless(ast.Boolean(false)) | ||
|
|
||
| err := encoder.Encode((*parser.Policy)(policy)) | ||
| testutil.Error(t, err) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,9 +74,13 @@ func (t Token) intValue() (int64, error) { | |
| } | ||
|
|
||
| func Tokenize(src []byte) ([]Token, error) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd be fine just changing the signature of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my comment about a new PR. |
||
| return TokenizeReader(bytes.NewBuffer(src)) | ||
| } | ||
|
|
||
| func TokenizeReader(r io.Reader) ([]Token, error) { | ||
| var res []Token | ||
| var s scanner | ||
| s.Init(bytes.NewBuffer(src)) | ||
| s.Init(r) | ||
| for tok := s.nextToken(); s.err == nil && tok.Type != TokenEOF; tok = s.nextToken() { | ||
| res = append(res, tok) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,8 +2,10 @@ package parser | |
|
|
||
| import ( | ||
| "fmt" | ||
| "io" | ||
| "strconv" | ||
| "strings" | ||
| "sync" | ||
|
|
||
| "github.com/cedar-policy/cedar-go/internal/consts" | ||
| "github.com/cedar-policy/cedar-go/internal/extensions" | ||
|
|
@@ -33,6 +35,38 @@ func (p *PolicySlice) UnmarshalCedar(b []byte) error { | |
| return nil | ||
| } | ||
|
|
||
| type Decoder struct { | ||
sagikazarmark marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| createParser func() (*parser, error) | ||
| } | ||
|
|
||
| func NewDecoder(r io.Reader) *Decoder { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @patjakdev @sagikazarmark How's this for a variation, I think it handles the errors how I want, does what @sagikazarmark needs and avoids using sync like @patjakdev wants:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the tokenizer gets rewritten anyway, I don't think it makes much of a difference (except maybe that the current solution is more idiomatic IMO). I can make the change if you want me to, but my vote goes to the current version. (I wouldn't mind getting this one merged and iterate later now that I managed to get the CI green)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're going to be serious about ensuring the same error is always returned from You could also inline
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That said, I'm fine with what you've got right now and happy to merge it. |
||
| return &Decoder{ | ||
| createParser: sync.OnceValues(func() (*parser, error) { | ||
| tokens, err := TokenizeReader(r) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| p := newParser(tokens) | ||
|
|
||
| return &p, nil | ||
| }), | ||
| } | ||
| } | ||
|
|
||
| func (d *Decoder) Decode(p *Policy) error { | ||
| parser, err := d.createParser() | ||
| if err != nil { | ||
philhassey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return err | ||
| } | ||
|
|
||
| if parser.peek().isEOF() { | ||
| return io.EOF | ||
| } | ||
|
|
||
| return p.fromCedar(parser) | ||
| } | ||
|
|
||
| func (p *Policy) UnmarshalCedar(b []byte) error { | ||
| tokens, err := Tokenize(b) | ||
| if err != nil { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package cedar | ||
|
|
||
| import ( | ||
| "io" | ||
|
|
||
| "github.com/cedar-policy/cedar-go/ast" | ||
| "github.com/cedar-policy/cedar-go/internal/parser" | ||
| ) | ||
|
|
||
| // Encoder encodes [Policy] statements in the human-readable format specified by the [Cedar documentation] | ||
| // and writes them to an [io.Writer]. | ||
| // | ||
| // [Cedar documentation]: https://docs.cedarpolicy.com/policies/syntax-grammar.html | ||
| type Encoder struct { | ||
| enc *parser.Encoder | ||
| } | ||
|
|
||
| // NewEncoder returns a new [Encoder]. | ||
| func NewEncoder(w io.Writer) *Encoder { | ||
| return &Encoder{enc: parser.NewEncoder(w)} | ||
| } | ||
|
|
||
| // Encode encodes and writes a single [Policy] statement to the underlying [io.Writer]. | ||
| func (e *Encoder) Encode(p *Policy) error { | ||
| return e.enc.Encode((*parser.Policy)(p.AST())) | ||
| } | ||
|
|
||
| // Decoder reads, parses and compiles [Policy] statements in the human-readable format specified by the [Cedar documentation] | ||
| // from an [io.Reader]. | ||
| // | ||
| // [Cedar documentation]: https://docs.cedarpolicy.com/policies/syntax-grammar.html | ||
| type Decoder struct { | ||
| dec *parser.Decoder | ||
| } | ||
|
|
||
| // NewDecoder returns a new [Decoder]. | ||
| func NewDecoder(r io.Reader) *Decoder { | ||
| return &Decoder{dec: parser.NewDecoder(r)} | ||
| } | ||
|
|
||
| // Decode parses and compiles a single [Policy] statement from the underlying [io.Reader]. | ||
| func (e *Decoder) Decode(p *Policy) error { | ||
| var parserPolicy parser.Policy | ||
|
|
||
| err := e.dec.Decode(&parserPolicy) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| *p = *NewPolicyFromAST((*ast.Policy)(&parserPolicy)) | ||
|
|
||
| return nil | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.