Skip to content

Commit

Permalink
refactor(filters): Consolidate property filters logic to a single func.
Browse files Browse the repository at this point in the history
Instead of having both "configurePropertyFiltersWithAllowedValues" and "getConfiguredPropertyFilters".
  • Loading branch information
rkettelerij committed Sep 16, 2024
1 parent cb91198 commit d713c67
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 30 deletions.
5 changes: 3 additions & 2 deletions internal/ogc/features/datasources/datasource.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/go-spatial/geom"
)

// Datasource holds all Features for a single object type in a specific projection.
// Datasource holds all Features for a single object type in a specific projection/CRS.
// This abstraction allows the rest of the system to stay datastore agnostic.
type Datasource interface {

Expand All @@ -29,7 +29,8 @@ type Datasource interface {
// GetFeatureTableMetadata returns metadata about a feature table associated with the given collection
GetFeatureTableMetadata(collection string) (FeatureTableMetadata, error)

// GetPropertyFiltersWithAllowedValues returns configured property filters for the given collection enriched with allowed values
// GetPropertyFiltersWithAllowedValues returns configured property filters for the given collection enriched with allowed values.
// When enrichments don't apply the returned result should still contain all property filters as specified in the (YAML) config.
GetPropertyFiltersWithAllowedValues(collection string) PropertyFiltersWithAllowedValues

// Close closes (connections to) the datasource gracefully
Expand Down
33 changes: 21 additions & 12 deletions internal/ogc/features/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ type Features struct {
}

func NewFeatures(e *engine.Engine) *Features {
configuredCollections = cacheConfiguredFeatureCollections(e)
datasources := createDatasources(e)
configuredPropertyFilters := configurePropertyFiltersWithAllowedValues(datasources)
configuredCollections = cacheConfiguredFeatureCollections(e)
configuredPropertyFilters := configurePropertyFiltersWithAllowedValues(datasources, configuredCollections)

rebuildOpenAPIForFeatures(e, datasources, configuredPropertyFilters)

Expand Down Expand Up @@ -94,7 +94,7 @@ func (f *Features) Features() http.HandlerFunc {
return
}
url := featureCollectionURL{*cfg.BaseURL.URL, r.URL.Query(), cfg.OgcAPI.Features.Limit,
getConfiguredPropertyFilters(collection), hasDateTime(collection)}
f.configuredPropertyFilters[collectionID], hasDateTime(collection)}
encodedCursor, limit, inputSRID, outputSRID, contentCrs, bbox, referenceDate, propertyFilters, err := url.parse()
if err != nil {
engine.RenderProblem(engine.ProblemBadRequest, w, err.Error())
Expand Down Expand Up @@ -275,10 +275,26 @@ func createDatasources(e *engine.Engine) map[DatasourceKey]ds.Datasource {
return result
}

func configurePropertyFiltersWithAllowedValues(datasources map[DatasourceKey]ds.Datasource) map[string]ds.PropertyFiltersWithAllowedValues {
func configurePropertyFiltersWithAllowedValues(datasources map[DatasourceKey]ds.Datasource,
collections map[string]config.GeoSpatialCollection) map[string]ds.PropertyFiltersWithAllowedValues {

result := make(map[string]ds.PropertyFiltersWithAllowedValues)
nrOfFilters := 0
for k, datasource := range datasources {
result[k.collectionID] = datasource.GetPropertyFiltersWithAllowedValues(k.collectionID)
filters := datasource.GetPropertyFiltersWithAllowedValues(k.collectionID)
result[k.collectionID] = filters
nrOfFilters += len(filters)
}

// sanity check to make sure datasources return all configured property filters.
nrOfExpectedFilters := 0
for _, collection := range collections {
if collection.Features != nil && collection.Features.Filters.Properties != nil {
nrOfExpectedFilters += len(collection.Features.Filters.Properties)
}
}
if nrOfExpectedFilters != nrOfFilters {
log.Fatal("number of property filters received from datasource does not match the number of configured property filters")
}
return result
}
Expand Down Expand Up @@ -385,13 +401,6 @@ func querySingleDatasource(input domain.SRID, output domain.SRID, bbox *geom.Ext
(int(input) == domain.WGS84SRID && int(output) == domain.UndefinedSRID)
}

func getConfiguredPropertyFilters(collection config.GeoSpatialCollection) []config.PropertyFilter {
if collection.Features != nil && collection.Features.Filters.Properties != nil {
return collection.Features.Filters.Properties
}
return []config.PropertyFilter{}
}

func getTemporalCriteria(collection config.GeoSpatialCollection, referenceDate time.Time) ds.TemporalCriteria {
var temporalCriteria ds.TemporalCriteria
if hasDateTime(collection) {
Expand Down
19 changes: 10 additions & 9 deletions internal/ogc/features/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/PDOK/gokoala/config"
"github.com/PDOK/gokoala/internal/engine"
"github.com/PDOK/gokoala/internal/ogc/features/datasources"
d "github.com/PDOK/gokoala/internal/ogc/features/domain"
"github.com/go-spatial/geom"
)
Expand Down Expand Up @@ -41,7 +42,7 @@ type featureCollectionURL struct {
baseURL url.URL
params url.Values
limit config.Limit
configuredPropertyFilters []config.PropertyFilter
configuredPropertyFilters map[string]datasources.PropertyFilterWithAllowedValues
supportsDatetime bool
}

Expand Down Expand Up @@ -134,8 +135,8 @@ func (fc featureCollectionURL) validateNoUnknownParams() error {
copyParams.Del(bboxCrsParam)
copyParams.Del(filterParam)
copyParams.Del(filterCrsParam)
for _, pf := range fc.configuredPropertyFilters {
copyParams.Del(pf.Name)
for pf := range fc.configuredPropertyFilters {
copyParams.Del(pf)
}
if len(copyParams) > 0 {
return fmt.Errorf("unknown query parameter(s) found: %v", copyParams.Encode())
Expand Down Expand Up @@ -291,22 +292,22 @@ func parseCrsToSRID(params url.Values, paramName string) (d.SRID, error) {
}

// Support simple filtering on properties: https://docs.ogc.org/is/17-069r4/17-069r4.html#_parameters_for_filtering_on_feature_properties
func parsePropertyFilters(configuredPropertyFilters []config.PropertyFilter, params url.Values) (map[string]string, error) {
func parsePropertyFilters(configuredPropertyFilters map[string]datasources.PropertyFilterWithAllowedValues, params url.Values) (map[string]string, error) {
propertyFilters := make(map[string]string)
for _, cpf := range configuredPropertyFilters {
pf := params.Get(cpf.Name)
for name := range configuredPropertyFilters {
pf := params.Get(name)
if pf != "" {
if len(pf) > propertyFilterMaxLength {
return nil, fmt.Errorf("property filter %s is too large, "+
"value is limited to %d characters", cpf.Name, propertyFilterMaxLength)
"value is limited to %d characters", name, propertyFilterMaxLength)
}
if strings.Contains(pf, propertyFilterWildcard) {
// if/when we choose to support wildcards in the future, make sure wildcards are
// only allowed at the END (suffix) of the filter
return nil, fmt.Errorf("property filter %s contains a wildcard (%s), "+
"wildcard filtering is not allowed", cpf.Name, propertyFilterWildcard)
"wildcard filtering is not allowed", name, propertyFilterWildcard)
}
propertyFilters[cpf.Name] = pf
propertyFilters[name] = pf
}
}
return propertyFilters, nil
Expand Down
15 changes: 8 additions & 7 deletions internal/ogc/features/url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/PDOK/gokoala/config"
"github.com/PDOK/gokoala/internal/ogc/features/datasources"

"github.com/PDOK/gokoala/internal/ogc/features/domain"
"github.com/go-spatial/geom"
Expand Down Expand Up @@ -472,14 +473,14 @@ func Test_featureCollectionURL_parseParams(t *testing.T) {
baseURL: tt.fields.baseURL,
params: tt.fields.params,
limit: tt.fields.limit,
configuredPropertyFilters: []config.PropertyFilter{
{
Name: "foo",
Description: "awesome foo property to filter on",
configuredPropertyFilters: map[string]datasources.PropertyFilterWithAllowedValues{
"foo": {
PropertyFilter: config.PropertyFilter{Name: "foo", Description: "awesome foo property to filter on"},
AllowedValues: nil,
},
{
Name: "bar",
Description: "even more awesome bar property to filter on",
"bar": {
PropertyFilter: config.PropertyFilter{Name: "bar", Description: "even more awesome bar property to filter on"},
AllowedValues: nil,
},
},
supportsDatetime: tt.fields.dtSupport,
Expand Down

0 comments on commit d713c67

Please sign in to comment.