From 620692b608e7e1fe7c81e9d3a3835050b1fcaf14 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 14 Mar 2024 13:00:34 +0100 Subject: [PATCH 1/3] Add optional.ExcractValue() to be able to handle Option type within interface{} value --- modules/optional/option.go | 28 +++++++++++++++++++++ modules/optional/option_test.go | 43 +++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/modules/optional/option.go b/modules/optional/option.go index af9e5ac852916..865b0206503ba 100644 --- a/modules/optional/option.go +++ b/modules/optional/option.go @@ -3,6 +3,8 @@ package optional +import "reflect" + type Option[T any] []T func None[T any]() Option[T] { @@ -43,3 +45,29 @@ func (o Option[T]) ValueOrDefault(v T) T { } return v } + +// ExcractValue return value or nil and bool if object was an Optional +// it should only be used if you already have to deal with interface{} values +// and expect an Option type within it. +func ExcractValue(obj any) (any, bool) { + rt := reflect.TypeOf(obj) + if rt.Kind() != reflect.Slice { + return nil, false + } + + type hasHasFunc interface { + Has() bool + } + if hasObj, ok := obj.(hasHasFunc); !ok { + return nil, false + } else if !hasObj.Has() { + return nil, true + } + + rv := reflect.ValueOf(obj) + if rv.Len() != 1 { + // it's still false as optional.Option[T] types would have reported with hasObj.Has() that it is empty + return nil, false + } + return rv.Index(0).Interface(), true +} diff --git a/modules/optional/option_test.go b/modules/optional/option_test.go index 4f55608004f87..32ac0ac0449f7 100644 --- a/modules/optional/option_test.go +++ b/modules/optional/option_test.go @@ -57,3 +57,46 @@ func TestOption(t *testing.T) { assert.True(t, opt3.Has()) assert.Equal(t, int(1), opt3.Value()) } + +func TestExcractValue(t *testing.T) { + val, ok := optional.ExcractValue("aaaa") + assert.False(t, ok) + assert.Nil(t, val) + + val, ok = optional.ExcractValue(optional.Some("aaaa")) + assert.True(t, ok) + if assert.NotNil(t, val) { + val, ok := val.(string) + assert.True(t, ok) + assert.EqualValues(t, "aaaa", val) + } + + val, ok = optional.ExcractValue(optional.None[float64]()) + assert.True(t, ok) + assert.Nil(t, val) + + val, ok = optional.ExcractValue(&fakeHas{}) + assert.False(t, ok) + assert.Nil(t, val) + + wrongType := make(fakeHas2, 0, 1) + val, ok = optional.ExcractValue(wrongType) + assert.False(t, ok) + assert.Nil(t, val) +} + +func toPtr[T any](val T) *T { + return &val +} + +type fakeHas struct{} + +func (fakeHas) Has() bool { + return true +} + +type fakeHas2 []string + +func (fakeHas2) Has() bool { + return true +} From ccc03ef0f7292e85e276d182ae7f67b167de7572 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 14 Mar 2024 13:32:03 +0100 Subject: [PATCH 2/3] Apply suggestions from code review --- modules/optional/option.go | 4 ++-- modules/optional/option_test.go | 16 ++++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/modules/optional/option.go b/modules/optional/option.go index 865b0206503ba..b134e36636ce8 100644 --- a/modules/optional/option.go +++ b/modules/optional/option.go @@ -46,10 +46,10 @@ func (o Option[T]) ValueOrDefault(v T) T { return v } -// ExcractValue return value or nil and bool if object was an Optional +// ExtractValue return value or nil and bool if object was an Optional // it should only be used if you already have to deal with interface{} values // and expect an Option type within it. -func ExcractValue(obj any) (any, bool) { +func ExtractValue(obj any) (any, bool) { rt := reflect.TypeOf(obj) if rt.Kind() != reflect.Slice { return nil, false diff --git a/modules/optional/option_test.go b/modules/optional/option_test.go index 32ac0ac0449f7..b7aaf773b7397 100644 --- a/modules/optional/option_test.go +++ b/modules/optional/option_test.go @@ -58,12 +58,12 @@ func TestOption(t *testing.T) { assert.Equal(t, int(1), opt3.Value()) } -func TestExcractValue(t *testing.T) { - val, ok := optional.ExcractValue("aaaa") +func TestExtractValue(t *testing.T) { + val, ok := optional.ExtractValue("aaaa") assert.False(t, ok) assert.Nil(t, val) - val, ok = optional.ExcractValue(optional.Some("aaaa")) + val, ok = optional.ExtractValue(optional.Some("aaaa")) assert.True(t, ok) if assert.NotNil(t, val) { val, ok := val.(string) @@ -71,24 +71,20 @@ func TestExcractValue(t *testing.T) { assert.EqualValues(t, "aaaa", val) } - val, ok = optional.ExcractValue(optional.None[float64]()) + val, ok = optional.ExtractValue(optional.None[float64]()) assert.True(t, ok) assert.Nil(t, val) - val, ok = optional.ExcractValue(&fakeHas{}) + val, ok = optional.ExtractValue(&fakeHas{}) assert.False(t, ok) assert.Nil(t, val) wrongType := make(fakeHas2, 0, 1) - val, ok = optional.ExcractValue(wrongType) + val, ok = optional.ExtractValue(wrongType) assert.False(t, ok) assert.Nil(t, val) } -func toPtr[T any](val T) *T { - return &val -} - type fakeHas struct{} func (fakeHas) Has() bool { From 511be113752880771206a1e40240adede72ef482 Mon Sep 17 00:00:00 2001 From: "m.huber" Date: Sat, 16 Mar 2024 00:10:15 +0100 Subject: [PATCH 3/3] make pagination.AddParam() optional.Option type aware --- services/context/pagination.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/services/context/pagination.go b/services/context/pagination.go index 68237c630c0ae..f225cb8ebf236 100644 --- a/services/context/pagination.go +++ b/services/context/pagination.go @@ -9,6 +9,7 @@ import ( "net/url" "strings" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/paginator" ) @@ -28,11 +29,20 @@ func NewPagination(total, pagingNum, current, numPages int) *Pagination { // AddParam adds a value from context identified by ctxKey as link param under a given paramKey func (p *Pagination) AddParam(ctx *Context, paramKey, ctxKey string) { - _, exists := ctx.Data[ctxKey] + obj, exists := ctx.Data[ctxKey] if !exists { return } - paramData := fmt.Sprintf("%v", ctx.Data[ctxKey]) // cast any to string + // we check if the value in the context is an optional.Option type and either skip if it contains None + // or unwrap it if it is Some + if optVal, is := optional.ExtractValue(obj); is { + if optVal == nil { + // optional value is currently None + return + } + obj = optVal + } + paramData := fmt.Sprintf("%v", obj) // cast any to string urlParam := fmt.Sprintf("%s=%v", url.QueryEscape(paramKey), url.QueryEscape(paramData)) p.urlParams = append(p.urlParams, urlParam) }