einride / aip-go

Go SDK for implementing resource-oriented gRPC APIs.
https://aip.dev
MIT License
157 stars 16 forks source link

TypeDuration is not parsed correctly #229

Open jan-sykora opened 1 year ago

jan-sykora commented 1 year ago

According to https://google.aip.dev/160

Durations expect a numeric representation followed by an s suffix (for seconds). Examples: 20s, 1.2s.

However, the aip-go parser doesn't parse this correctly.

For example, we have defined a field called some_duration. This field is declared with type filtering.TypeDuration.

If we want to filter all resources with some_duration equal to 7 seconds, then the filter expression should be: some_duration = 7s. However, when parsing this expression using the above mentioned declaration, we get an error:

some_duration = 7s
                 ^ unexpected trailing token TEXT (1:18)

Iit seems that parser accepts filter expression in this form: some_duration = duration(7s). However, this is against the https://google.aip.dev/160.

Fully working example

package main

import (
    "go.einride.tech/aip/filtering"
)

type ListRequest struct {
    filter string
}

func (r *ListRequest) GetFilter() string {
    return r.filter
}

func main() {
    declarations, _ := filtering.NewDeclarations(
        filtering.DeclareStandardFunctions(),
        filtering.DeclareIdent("some_duration", filtering.TypeDuration),
    )

    req := &ListRequest{
        filter: "some_duration = 7s", // will fail: unexpected trailing token TEXT (1:18)
        //filter: "some_duration = \"7s\"", // will fail: no matching overload found for calling '=' with [well_known:DURATION primitive:STRING]
        //filter: "some_duration = duration(7s)", // will fail: unexpected trailing token ( (1:25)
        //filter: "some_duration = duration(\"7s\")", // will pass
    }

    _, err := filtering.ParseFilter(req, declarations)
    if err != nil {
        panic(err)
    }
}

Workaround

As a workaround, I am handling the duration value as a TypeString and I retrieve the duration value from string value when traversing the parsed tree:

func main() {
    declarations, _ := filtering.NewDeclarations(
        filtering.DeclareStandardFunctions(),
        filtering.DeclareIdent("some_duration", filtering.TypeString),
    )

    req := &ListRequest{ filter: "some_duration = \"7s\"" }

    aipFilter, err := filtering.ParseFilter(req, declarations)
    assert.NoError(err)

    // Walk through the parsed aipFilter tree and when finding some_duration ident, then convert "7s" string to duration
    // Need to manually check correct format of the duration, because now the value can be an arbitrary string.
}
github-actions[bot] commented 3 months ago

This issue has been open for 365 days with no activity. Marking as stale.