aip-go icon indicating copy to clipboard operation
aip-go copied to clipboard

TypeDuration is not parsed correctly

Open jan-sykora opened this issue 1 year ago • 1 comments

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.
}

jan-sykora avatar May 11 '23 16:05 jan-sykora