Merge pull request #120 from qubic/feature/improve-query

Feature/improve query

Author: qubicmio

.github/ISSUE_TEMPLATE/bug_report.md

@@ -10,7 +10,7 @@ assignees: ''
 **Describe the bug**
 A clear and concise description of what the bug is.
 
-**To Reproduce**
+**How to reproduce**
 Steps to reproduce the behavior.
 
 1. '...'

v2/domain/repository/elastic/transaction.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"log"
 	"sort"
+	"strconv"
 	"strings"
 
 	api "github.com/qubic/archive-query-service/v2/api/archive-query-service/v2"
@@ -166,9 +167,17 @@ func createIdentitiesQuery(identity string, filters map[string][]string, ranges
 
 	includeFilters, excludeFilters := splitFilters(filters)
 
+	// Check if there's an upper bound tickNumber range filter (lt/lte) and adjust if needed
+	hasUpperBoundTickFilter, err := modifyUpperBoundTickNumberFilterIfNecessary(ranges, maxTick)
+	if err != nil {
+		return "", err
+	}
+
 	filterStrings := make([]string, 0, len(includeFilters)+len(ranges)+1)
-	// restrict to max tick (we don't care about potential duplicate tickNumber range filter)
-	filterStrings = append(filterStrings, fmt.Sprintf(`{"range":{"tickNumber":{"lte":"%d"}}}`, maxTick))
+	// restrict to max tick only if no upper bound tickNumber filter is present
+	if !hasUpperBoundTickFilter {
+		filterStrings = append(filterStrings, fmt.Sprintf(`{"range":{"tickNumber":{"lte":"%d"}}}`, maxTick))
+	}
 	// normal filters
 	filterStrings = append(filterStrings, getFilterStrings(includeFilters)...)
 	// append range filters
@@ -181,6 +190,15 @@ func createIdentitiesQuery(identity string, filters map[string][]string, ranges
 	// filters for excluding results
 	excludeFilterStrings := getFilterStrings(excludeFilters)
 
+	// filters are always present because we need to restrict to max tick
+	filterQueryString := strings.Join(filterStrings, ",")
+
+	// only add exclude filters if present (otherwise empty)
+	mustNotQueryString := strings.Join(excludeFilterStrings, ",")
+	if len(mustNotQueryString) > 0 {
+		mustNotQueryString = fmt.Sprintf(`, "must_not": [ %s ]`, mustNotQueryString)
+	}
+
 	// in case we have a source or destination filter, the should clause still works
 	query = `{ 
       "query": {
@@ -190,8 +208,7 @@ func createIdentitiesQuery(identity string, filters map[string][]string, ranges
 			{ "term":{"destination":"%s"} }
 		  ],
 		  "minimum_should_match": 1,
-		  "filter": [ %s ],
-          "must_not": [ %s ]
+		  "filter": [ %s ] %s
 		}
 	  },
 	  "sort": [ {"tickNumber":{"order":"desc"}} ],
@@ -201,11 +218,39 @@ func createIdentitiesQuery(identity string, filters map[string][]string, ranges
 	}`
 
 	query = fmt.Sprintf(query, identity, identity,
-		strings.Join(filterStrings, ","), strings.Join(excludeFilterStrings, ","),
+		filterQueryString, mustNotQueryString,
 		from, size, maxTrackTotalHits)
 	return query, nil
 }
 
+func modifyUpperBoundTickNumberFilterIfNecessary(ranges map[string][]*entities.Range, maxTick uint32) (bool, error) {
+	hasUpperBoundTickFilter := false
+	if tickRanges, ok := ranges["tickNumber"]; ok {
+		for _, r := range tickRanges {
+			if r.Operation == "lt" || r.Operation == "lte" {
+				hasUpperBoundTickFilter = true
+				// Parse the value and compare with maxTick
+				tickValue, err := strconv.ParseUint(r.Value, 10, 32)
+				if err != nil {
+					return false, fmt.Errorf("parsing tickNumber range value: %w", err)
+				}
+				maxTickValue := If(r.Operation == "lte", maxTick, maxTick+1)
+				if uint32(tickValue) > maxTickValue {
+					r.Value = fmt.Sprintf("%d", maxTickValue)
+				}
+			}
+		}
+	}
+	return hasUpperBoundTickFilter, nil
+}
+
+func If[T any](cond bool, vtrue, vfalse T) T {
+	if cond {
+		return vtrue
+	}
+	return vfalse
+}
+
 const excludeSuffix = "-exclude"
 
 func splitFilters(filters map[string][]string) (map[string][]string, map[string][]string) {

v2/domain/repository/elastic/transaction_identities_query_test.go

@@ -19,8 +19,7 @@ func Test_createIdentitiesQuery_returnQuery(t *testing.T) {
 			{ "term":{"destination":"some-identity"} }
 		  ],
 		  "minimum_should_match": 1,
-		  "filter": [{"range":{"tickNumber":{"lte":"12345"}}}],
-		  "must_not": []
+		  "filter": [{"range":{"tickNumber":{"lte":"12345"}}}]
 		}
 	  },
 	  "sort": [ {"tickNumber":{"order":"desc"}} ],
@@ -49,8 +48,7 @@ func Test_createIdentitiesQuery_givenFilters_returnQueryWithFilters(t *testing.T
             {"range":{"tickNumber":{"lte":"1000000"}}},
 			{"term":{"another-value":"foo"}},
 			{"term":{"some-value":"42"}}
-          ],
-		  "must_not": []
+          ]
 		}
 	  },
 	  "sort": [ {"tickNumber":{"order":"desc"}} ],
@@ -113,8 +111,7 @@ func Test_createIdentitiesQuery_givenRanges_returnQueryWithFilters(t *testing.T)
 			{"range":{"another-value":{ "lte": "43", "gte": "12"  }}},
 			{"range":{"some-value":{ "lt": "42" }}},
 			{"range":{"third-value":{ "gt": "44"}}}
-          ],
-		  "must_not": []
+          ]
 		}
 	  },
 	  "sort": [ {"tickNumber":{"order":"desc"}} ],
@@ -171,3 +168,201 @@ func Test_createIdentitiesQuery_givenRangesAndFilters_returnQueryWithAllFilters(
 	log.Println(query)
 	require.JSONEq(t, expectedQuery, query)
 }
+
+func Test_modifyUpperBoundTickNumberFilterIfNecessary_noRangeFilter(t *testing.T) {
+	ranges := map[string][]*entities.Range{}
+	maxTick := uint32(1000)
+
+	hasUpperBound, err := modifyUpperBoundTickNumberFilterIfNecessary(ranges, maxTick)
+	require.NoError(t, err)
+	require.False(t, hasUpperBound)
+}
+
+func Test_modifyUpperBoundTickNumberFilterIfNecessary_upperBoundReplacedWithMaxTick(t *testing.T) {
+	ranges := map[string][]*entities.Range{
+		"tickNumber": {
+			{Operation: "lte", Value: "5000"},
+		},
+	}
+	maxTick := uint32(1000)
+
+	hasUpperBound, err := modifyUpperBoundTickNumberFilterIfNecessary(ranges, maxTick)
+	require.NoError(t, err)
+	require.True(t, hasUpperBound)
+	require.Equal(t, "1000", ranges["tickNumber"][0].Value)
+}
+
+func Test_modifyUpperBoundTickNumberFilterIfNecessary_upperBoundNotReplaced(t *testing.T) {
+	ranges := map[string][]*entities.Range{
+		"tickNumber": {
+			{Operation: "lte", Value: "999"},
+		},
+	}
+	maxTick := uint32(1000)
+
+	hasUpperBound, err := modifyUpperBoundTickNumberFilterIfNecessary(ranges, maxTick)
+	require.NoError(t, err)
+	require.True(t, hasUpperBound)
+	require.Equal(t, "999", ranges["tickNumber"][0].Value)
+}
+
+func Test_modifyUpperBoundTickNumberFilterIfNecessary_onlyLowerBound(t *testing.T) {
+	ranges := map[string][]*entities.Range{
+		"tickNumber": {
+			{Operation: "gte", Value: "100"},
+		},
+	}
+	maxTick := uint32(1000)
+
+	hasUpperBound, err := modifyUpperBoundTickNumberFilterIfNecessary(ranges, maxTick)
+	require.NoError(t, err)
+	require.False(t, hasUpperBound)
+	require.Equal(t, "100", ranges["tickNumber"][0].Value)
+}
+
+func Test_modifyUpperBoundTickNumberFilterIfNecessary_ltOperatorNotReplacedWithMaxTick(t *testing.T) {
+	ranges := map[string][]*entities.Range{
+		"tickNumber": {
+			{Operation: "lt", Value: "1001"},
+		},
+	}
+	maxTick := uint32(1000)
+
+	hasUpperBound, err := modifyUpperBoundTickNumberFilterIfNecessary(ranges, maxTick)
+	require.NoError(t, err)
+	require.True(t, hasUpperBound)
+	require.Equal(t, "1001", ranges["tickNumber"][0].Value)
+}
+
+func Test_modifyUpperBoundTickNumberFilterIfNecessary_ltOperatorReplacedWithMaxTick(t *testing.T) {
+	ranges := map[string][]*entities.Range{
+		"tickNumber": {
+			{Operation: "lt", Value: "1002"},
+		},
+	}
+	maxTick := uint32(1000)
+
+	hasUpperBound, err := modifyUpperBoundTickNumberFilterIfNecessary(ranges, maxTick)
+	require.NoError(t, err)
+	require.True(t, hasUpperBound)
+	require.Equal(t, "1001", ranges["tickNumber"][0].Value)
+}
+
+func Test_modifyUpperBoundTickNumberFilterIfNecessary_lteOperatorReplacedWithMaxTick(t *testing.T) {
+	ranges := map[string][]*entities.Range{
+		"tickNumber": {
+			{Operation: "lte", Value: "5000"},
+		},
+	}
+	maxTick := uint32(1000)
+
+	hasUpperBound, err := modifyUpperBoundTickNumberFilterIfNecessary(ranges, maxTick)
+	require.NoError(t, err)
+	require.True(t, hasUpperBound)
+	require.Equal(t, "1000", ranges["tickNumber"][0].Value)
+}
+
+func Test_modifyUpperBoundTickNumberFilterIfNecessary_invalidValueReturnsError(t *testing.T) {
+	ranges := map[string][]*entities.Range{
+		"tickNumber": {
+			{Operation: "lte", Value: "not-a-number"},
+		},
+	}
+	maxTick := uint32(1000)
+
+	hasUpperBound, err := modifyUpperBoundTickNumberFilterIfNecessary(ranges, maxTick)
+	require.Error(t, err)
+	require.False(t, hasUpperBound)
+	require.Contains(t, err.Error(), "parsing tickNumber range value")
+}
+
+func Test_createIdentitiesQuery_withTickNumberUpperBound_omitsMaxTickFilter(t *testing.T) {
+	expectedQuery := `{
+      "query": {
+		"bool": {
+		  "should": [
+			{ "term":{"source":"some-identity"} },
+			{ "term":{"destination":"some-identity"} }
+		  ],
+		  "minimum_should_match": 1,
+		  "filter": [
+			{"range":{"tickNumber":{"lte":"500"}}}
+          ]
+		}
+	  },
+	  "sort": [ {"tickNumber":{"order":"desc"}} ],
+	  "from": 0,
+	  "size": 10,
+	  "track_total_hits": 10000
+	}`
+
+	ranges := map[string][]*entities.Range{
+		"tickNumber": {{Operation: "lte", Value: "500"}},
+	}
+	query, err := createIdentitiesQuery(testIdentity, nil, ranges, 0, 10, 1000)
+	require.NoError(t, err)
+	require.NotEmpty(t, query)
+
+	require.JSONEq(t, expectedQuery, query)
+}
+
+func Test_createIdentitiesQuery_withTickNumberUpperBoundExceedingMaxTick_replacesWithMaxTick(t *testing.T) {
+	expectedQuery := `{
+      "query": {
+		"bool": {
+		  "should": [
+			{ "term":{"source":"some-identity"} },
+			{ "term":{"destination":"some-identity"} }
+		  ],
+		  "minimum_should_match": 1,
+		  "filter": [
+			{"range":{"tickNumber":{"lt":"1001"}}}
+          ]
+		}
+	  },
+	  "sort": [ {"tickNumber":{"order":"desc"}} ],
+	  "from": 0,
+	  "size": 10,
+	  "track_total_hits": 10000
+	}`
+
+	ranges := map[string][]*entities.Range{
+		"tickNumber": {{Operation: "lt", Value: "5000"}},
+	}
+	query, err := createIdentitiesQuery(testIdentity, nil, ranges, 0, 10, 1000)
+	require.NoError(t, err)
+	require.NotEmpty(t, query)
+
+	require.JSONEq(t, expectedQuery, query)
+}
+
+func Test_createIdentitiesQuery_withTickNumberLowerBoundOnly_includesMaxTickFilter(t *testing.T) {
+	expectedQuery := `{
+      "query": {
+		"bool": {
+		  "should": [
+			{ "term":{"source":"some-identity"} },
+			{ "term":{"destination":"some-identity"} }
+		  ],
+		  "minimum_should_match": 1,
+		  "filter": [
+			{"range":{"tickNumber":{"lte":"1000"}}},
+			{"range":{"tickNumber":{"gte":"100"}}}
+          ]
+		}
+	  },
+	  "sort": [ {"tickNumber":{"order":"desc"}} ],
+	  "from": 0,
+	  "size": 10,
+	  "track_total_hits": 10000
+	}`
+
+	ranges := map[string][]*entities.Range{
+		"tickNumber": {{Operation: "gte", Value: "100"}},
+	}
+	query, err := createIdentitiesQuery(testIdentity, nil, ranges, 0, 10, 1000)
+	require.NoError(t, err)
+	require.NotEmpty(t, query)
+
+	require.JSONEq(t, expectedQuery, query)
+}

v2/domain/repository/elastic/transaction_range_filter_test.go

@@ -0,0 +1,42 @@
+package elastic
+
+import (
+	"testing"
+
+	"github.com/qubic/archive-query-service/v2/entities"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_createRangeFilter(t *testing.T) {
+	tests := []struct {
+		name     string
+		property string
+		ranges   []*entities.Range
+		want     string
+	}{
+		{
+			name:     "valid single range",
+			property: "tick",
+			ranges: []*entities.Range{
+				{Operation: "gte", Value: "100"},
+			},
+			want: `{"range":{"tick":{"gte":"100"}}}`,
+		},
+		{
+			name:     "valid dual range",
+			property: "tick",
+			ranges: []*entities.Range{
+				{Operation: "gte", Value: "100"},
+				{Operation: "lte", Value: "200"},
+			},
+			want: `{"range":{"tick":{"gte":"100","lte":"200"}}}`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := createRangeFilter(tt.property, tt.ranges)
+			require.NoError(t, err)
+			require.JSONEq(t, tt.want, got)
+		})
+	}
+}