diff --git a/internal/api/client/filters/v1/filterpost_test.go b/internal/api/client/filters/v1/filterpost_test.go index 2b18abf13..b7aecc573 100644 --- a/internal/api/client/filters/v1/filterpost_test.go +++ b/internal/api/client/filters/v1/filterpost_test.go @@ -41,6 +41,7 @@ func (suite *FiltersTestSuite) postFilter( irreversible *bool, wholeWord *bool, expiresIn *int, + expiresInStr *string, requestJson *string, expectedHTTPStatus int, expectedBody string, @@ -75,6 +76,8 @@ func (suite *FiltersTestSuite) postFilter( } if expiresIn != nil { ctx.Request.Form["expires_in"] = []string{strconv.Itoa(*expiresIn)} + } else if expiresInStr != nil { + ctx.Request.Form["expires_in"] = []string{*expiresInStr} } } @@ -124,7 +127,7 @@ func (suite *FiltersTestSuite) TestPostFilterFull() { irreversible := false wholeWord := true expiresIn := 86400 - filter, err := suite.postFilter(&phrase, &context, &irreversible, &wholeWord, &expiresIn, nil, http.StatusOK, "") + filter, err := suite.postFilter(&phrase, &context, &irreversible, &wholeWord, &expiresIn, nil, nil, http.StatusOK, "") if err != nil { suite.FailNow(err.Error()) } @@ -155,7 +158,7 @@ func (suite *FiltersTestSuite) TestPostFilterFullJSON() { "whole_word": true, "expires_in": 86400.1 }` - filter, err := suite.postFilter(nil, nil, nil, nil, nil, &requestJson, http.StatusOK, "") + filter, err := suite.postFilter(nil, nil, nil, nil, nil, nil, &requestJson, http.StatusOK, "") if err != nil { suite.FailNow(err.Error()) } @@ -182,7 +185,7 @@ func (suite *FiltersTestSuite) TestPostFilterMinimal() { phrase := "GNU/Linux" context := []string{"home"} - filter, err := suite.postFilter(&phrase, &context, nil, nil, nil, nil, http.StatusOK, "") + filter, err := suite.postFilter(&phrase, &context, nil, nil, nil, nil, nil, http.StatusOK, "") if err != nil { suite.FailNow(err.Error()) } @@ -203,7 +206,7 @@ func (suite *FiltersTestSuite) TestPostFilterMinimal() { func (suite *FiltersTestSuite) TestPostFilterEmptyPhrase() { phrase := "" context := []string{"home"} - _, err := suite.postFilter(&phrase, &context, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.postFilter(&phrase, &context, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") if err != nil { suite.FailNow(err.Error()) } @@ -211,7 +214,7 @@ func (suite *FiltersTestSuite) TestPostFilterEmptyPhrase() { func (suite *FiltersTestSuite) TestPostFilterMissingPhrase() { context := []string{"home"} - _, err := suite.postFilter(nil, &context, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.postFilter(nil, &context, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") if err != nil { suite.FailNow(err.Error()) } @@ -220,7 +223,7 @@ func (suite *FiltersTestSuite) TestPostFilterMissingPhrase() { func (suite *FiltersTestSuite) TestPostFilterEmptyContext() { phrase := "GNU/Linux" context := []string{} - _, err := suite.postFilter(&phrase, &context, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.postFilter(&phrase, &context, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") if err != nil { suite.FailNow(err.Error()) } @@ -228,7 +231,7 @@ func (suite *FiltersTestSuite) TestPostFilterEmptyContext() { func (suite *FiltersTestSuite) TestPostFilterMissingContext() { phrase := "GNU/Linux" - _, err := suite.postFilter(&phrase, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.postFilter(&phrase, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") if err != nil { suite.FailNow(err.Error()) } @@ -237,8 +240,37 @@ func (suite *FiltersTestSuite) TestPostFilterMissingContext() { // There should be a filter with this phrase as its title in our test fixtures. Creating another should fail. func (suite *FiltersTestSuite) TestPostFilterTitleConflict() { phrase := "fnord" - _, err := suite.postFilter(&phrase, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.postFilter(&phrase, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") if err != nil { suite.FailNow(err.Error()) } } + +// postFilterWithExpiration creates a filter with optional expiration. +func (suite *FiltersTestSuite) postFilterWithExpiration(phrase *string, expiresIn *int, expiresInStr *string, requestJson *string) *apimodel.FilterV1 { + context := []string{"home"} + filter, err := suite.postFilter(phrase, &context, nil, nil, expiresIn, expiresInStr, requestJson, http.StatusOK, "") + if err != nil { + suite.FailNow(err.Error()) + } + return filter +} + +// Regression test for https://github.com/superseriousbusiness/gotosocial/issues/3497 +func (suite *FiltersTestSuite) TestPostFilterWithEmptyStringExpiration() { + title := "Form Sins" + expiresInStr := "" + filter := suite.postFilterWithExpiration(&title, nil, &expiresInStr, nil) + suite.Nil(filter.ExpiresAt) +} + +// Regression test related to https://github.com/superseriousbusiness/gotosocial/issues/3497 +func (suite *FiltersTestSuite) TestPostFilterWithNullExpirationJSON() { + requestJson := `{ + "phrase": "JSON Sins", + "context": ["home"], + "expires_in": null + }` + filter := suite.postFilterWithExpiration(nil, nil, nil, &requestJson) + suite.Nil(filter.ExpiresAt) +} diff --git a/internal/api/client/filters/v1/filterput_test.go b/internal/api/client/filters/v1/filterput_test.go index 40b52ee43..626bd52eb 100644 --- a/internal/api/client/filters/v1/filterput_test.go +++ b/internal/api/client/filters/v1/filterput_test.go @@ -42,6 +42,7 @@ func (suite *FiltersTestSuite) putFilter( irreversible *bool, wholeWord *bool, expiresIn *int, + expiresInStr *string, requestJson *string, expectedHTTPStatus int, expectedBody string, @@ -76,6 +77,8 @@ func (suite *FiltersTestSuite) putFilter( } if expiresIn != nil { ctx.Request.Form["expires_in"] = []string{strconv.Itoa(*expiresIn)} + } else if expiresInStr != nil { + ctx.Request.Form["expires_in"] = []string{*expiresInStr} } } @@ -128,7 +131,7 @@ func (suite *FiltersTestSuite) TestPutFilterFull() { irreversible := false wholeWord := true expiresIn := 86400 - filter, err := suite.putFilter(id, &phrase, &context, &irreversible, &wholeWord, &expiresIn, nil, http.StatusOK, "") + filter, err := suite.putFilter(id, &phrase, &context, &irreversible, &wholeWord, &expiresIn, nil, nil, http.StatusOK, "") if err != nil { suite.FailNow(err.Error()) } @@ -160,7 +163,7 @@ func (suite *FiltersTestSuite) TestPutFilterFullJSON() { "whole_word": true, "expires_in": 86400.1 }` - filter, err := suite.putFilter(id, nil, nil, nil, nil, nil, &requestJson, http.StatusOK, "") + filter, err := suite.putFilter(id, nil, nil, nil, nil, nil, nil, &requestJson, http.StatusOK, "") if err != nil { suite.FailNow(err.Error()) } @@ -188,7 +191,7 @@ func (suite *FiltersTestSuite) TestPutFilterMinimal() { id := suite.testFilterKeywords["local_account_1_filter_1_keyword_1"].ID phrase := "GNU/Linux" context := []string{"home"} - filter, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, http.StatusOK, "") + filter, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, nil, http.StatusOK, "") if err != nil { suite.FailNow(err.Error()) } @@ -210,7 +213,7 @@ func (suite *FiltersTestSuite) TestPutFilterEmptyPhrase() { id := suite.testFilterKeywords["local_account_1_filter_1_keyword_1"].ID phrase := "" context := []string{"home"} - _, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") if err != nil { suite.FailNow(err.Error()) } @@ -219,7 +222,7 @@ func (suite *FiltersTestSuite) TestPutFilterEmptyPhrase() { func (suite *FiltersTestSuite) TestPutFilterMissingPhrase() { id := suite.testFilterKeywords["local_account_1_filter_1_keyword_1"].ID context := []string{"home"} - _, err := suite.putFilter(id, nil, &context, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.putFilter(id, nil, &context, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") if err != nil { suite.FailNow(err.Error()) } @@ -229,7 +232,7 @@ func (suite *FiltersTestSuite) TestPutFilterEmptyContext() { id := suite.testFilterKeywords["local_account_1_filter_1_keyword_1"].ID phrase := "GNU/Linux" context := []string{} - _, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") if err != nil { suite.FailNow(err.Error()) } @@ -238,7 +241,7 @@ func (suite *FiltersTestSuite) TestPutFilterEmptyContext() { func (suite *FiltersTestSuite) TestPutFilterMissingContext() { id := suite.testFilterKeywords["local_account_1_filter_1_keyword_1"].ID phrase := "GNU/Linux" - _, err := suite.putFilter(id, &phrase, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.putFilter(id, &phrase, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") if err != nil { suite.FailNow(err.Error()) } @@ -248,7 +251,7 @@ func (suite *FiltersTestSuite) TestPutFilterMissingContext() { func (suite *FiltersTestSuite) TestPutFilterTitleConflict() { id := suite.testFilterKeywords["local_account_1_filter_1_keyword_1"].ID phrase := "metasyntactic variables" - _, err := suite.putFilter(id, &phrase, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.putFilter(id, &phrase, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") if err != nil { suite.FailNow(err.Error()) } @@ -258,7 +261,7 @@ func (suite *FiltersTestSuite) TestPutAnotherAccountsFilter() { id := suite.testFilterKeywords["local_account_2_filter_1_keyword_1"].ID phrase := "GNU/Linux" context := []string{"home"} - _, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, http.StatusNotFound, `{"error":"Not Found"}`) + _, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, nil, http.StatusNotFound, `{"error":"Not Found"}`) if err != nil { suite.FailNow(err.Error()) } @@ -268,8 +271,60 @@ func (suite *FiltersTestSuite) TestPutNonexistentFilter() { id := "not_even_a_real_ULID" phrase := "GNU/Linux" context := []string{"home"} - _, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, http.StatusNotFound, `{"error":"Not Found"}`) + _, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, nil, http.StatusNotFound, `{"error":"Not Found"}`) if err != nil { suite.FailNow(err.Error()) } } + +// setFilterExpiration sets filter expiration. +func (suite *FiltersTestSuite) setFilterExpiration(id string, phrase *string, expiresIn *int, expiresInStr *string, requestJson *string) *apimodel.FilterV1 { + context := []string{"home"} + filter, err := suite.putFilter(id, phrase, &context, nil, nil, expiresIn, expiresInStr, requestJson, http.StatusOK, "") + if err != nil { + suite.FailNow(err.Error()) + } + return filter +} + +// Regression test for https://github.com/superseriousbusiness/gotosocial/issues/3497 +func (suite *FiltersTestSuite) TestPutFilterUnsetExpirationDateEmptyString() { + filterKeyword := suite.testFilterKeywords["local_account_1_filter_1_keyword_1"] + id := filterKeyword.ID + phrase := filterKeyword.Keyword + + // Setup: set an expiration date for the filter. + expiresIn := 86400 + filter := suite.setFilterExpiration(id, &phrase, &expiresIn, nil, nil) + if !suite.NotNil(filter.ExpiresAt) { + suite.FailNow("Test precondition failed") + } + + // Unset the filter's expiration date by setting it to an empty string. + expiresInStr := "" + filter = suite.setFilterExpiration(id, &phrase, nil, &expiresInStr, nil) + suite.Nil(filter.ExpiresAt) +} + +// Regression test related to https://github.com/superseriousbusiness/gotosocial/issues/3497 +func (suite *FiltersTestSuite) TestPutFilterUnsetExpirationDateNullJSON() { + filterKeyword := suite.testFilterKeywords["local_account_1_filter_1_keyword_1"] + id := filterKeyword.ID + phrase := filterKeyword.Keyword + + // Setup: set an expiration date for the filter. + expiresIn := 86400 + filter := suite.setFilterExpiration(id, &phrase, &expiresIn, nil, nil) + if !suite.NotNil(filter.ExpiresAt) { + suite.FailNow("Test precondition failed") + } + + // Unset the filter's expiration date by setting it to a null literal. + requestJson := `{ + "phrase": "fnord", + "context": ["home"], + "expires_in": null + }` + filter = suite.setFilterExpiration(id, nil, nil, nil, &requestJson) + suite.Nil(filter.ExpiresAt) +} diff --git a/internal/api/client/filters/v1/validate.go b/internal/api/client/filters/v1/validate.go index 9e876c8cf..9e31abb89 100644 --- a/internal/api/client/filters/v1/validate.go +++ b/internal/api/client/filters/v1/validate.go @@ -46,12 +46,11 @@ func validateNormalizeCreateUpdateFilter(form *apimodel.FilterCreateUpdateReques return errors.New("irreversible aka server-side drop filters are not supported yet") } - // Normalize filter expiry if necessary. - if form.ExpiresInI != nil { - // If we parsed this as JSON, expires_in - // may be either a float64 or a string. + // If `expires_in` was provided + // as JSON, then normalize it. + if form.ExpiresInI.IsSpecified() { var err error - form.ExpiresIn, err = apiutil.ParseDuration( + form.ExpiresIn, err = apiutil.ParseNullableDuration( form.ExpiresInI, "expires_in", ) @@ -60,10 +59,5 @@ func validateNormalizeCreateUpdateFilter(form *apimodel.FilterCreateUpdateReques } } - // Interpret zero as indefinite duration. - if form.ExpiresIn != nil && *form.ExpiresIn == 0 { - form.ExpiresIn = nil - } - return nil } diff --git a/internal/api/client/filters/v2/filterpost.go b/internal/api/client/filters/v2/filterpost.go index 632c4402f..ad6e83060 100644 --- a/internal/api/client/filters/v2/filterpost.go +++ b/internal/api/client/filters/v2/filterpost.go @@ -225,12 +225,11 @@ func validateNormalizeCreateFilter(form *apimodel.FilterCreateRequestV2) error { // Apply defaults for missing fields. form.FilterAction = util.Ptr(action) - // Normalize filter expiry if necessary. - if form.ExpiresInI != nil { - // If we parsed this as JSON, expires_in - // may be either a float64 or a string. + // If `expires_in` was provided + // as JSON, then normalize it. + if form.ExpiresInI.IsSpecified() { var err error - form.ExpiresIn, err = apiutil.ParseDuration( + form.ExpiresIn, err = apiutil.ParseNullableDuration( form.ExpiresInI, "expires_in", ) @@ -239,11 +238,6 @@ func validateNormalizeCreateFilter(form *apimodel.FilterCreateRequestV2) error { } } - // Interpret zero as indefinite duration. - if form.ExpiresIn != nil && *form.ExpiresIn == 0 { - form.ExpiresIn = nil - } - // Normalize and validate new keywords and statuses. for i, formKeyword := range form.Keywords { if err := validate.FilterKeyword(formKeyword.Keyword); err != nil { diff --git a/internal/api/client/filters/v2/filterpost_test.go b/internal/api/client/filters/v2/filterpost_test.go index 6e378874c..7a79f4665 100644 --- a/internal/api/client/filters/v2/filterpost_test.go +++ b/internal/api/client/filters/v2/filterpost_test.go @@ -36,7 +36,7 @@ "github.com/superseriousbusiness/gotosocial/testrig" ) -func (suite *FiltersTestSuite) postFilter(title *string, context *[]string, action *string, expiresIn *int, keywordsAttributesKeyword *[]string, keywordsAttributesWholeWord *[]bool, statusesAttributesStatusID *[]string, requestJson *string, expectedHTTPStatus int, expectedBody string) (*apimodel.FilterV2, error) { +func (suite *FiltersTestSuite) postFilter(title *string, context *[]string, action *string, expiresIn *int, expiresInStr *string, keywordsAttributesWholeWord *[]bool, statusesAttributesStatusID *[]string, requestJson *string, expectedHTTPStatus int, expectedBody string, keywordsAttributesKeyword *[]string) (*apimodel.FilterV2, error) { // instantiate recorder + test context recorder := httptest.NewRecorder() ctx, _ := testrig.CreateGinTestContext(recorder, nil) @@ -64,6 +64,8 @@ func (suite *FiltersTestSuite) postFilter(title *string, context *[]string, acti } if expiresIn != nil { ctx.Request.Form["expires_in"] = []string{strconv.Itoa(*expiresIn)} + } else if expiresInStr != nil { + ctx.Request.Form["expires_in"] = []string{*expiresInStr} } if keywordsAttributesKeyword != nil { ctx.Request.Form["keywords_attributes[][keyword]"] = *keywordsAttributesKeyword @@ -130,7 +132,7 @@ func (suite *FiltersTestSuite) TestPostFilterFull() { keywordsAttributesWholeWord := []bool{true, false} // Checked in lexical order by status ID, so keep this sorted. statusAttributesStatusID := []string{"01HEN2QRFA8H3C6QPN7RD4KSR6", "01HEWV37MHV8BAC8ANFGVRRM5D"} - filter, err := suite.postFilter(&title, &context, &action, &expiresIn, &keywordsAttributesKeyword, &keywordsAttributesWholeWord, &statusAttributesStatusID, nil, http.StatusOK, "") + filter, err := suite.postFilter(&title, &context, &action, &expiresIn, nil, &keywordsAttributesWholeWord, &statusAttributesStatusID, nil, http.StatusOK, "", &keywordsAttributesKeyword) if err != nil { suite.FailNow(err.Error()) } @@ -197,7 +199,7 @@ func (suite *FiltersTestSuite) TestPostFilterFullJSON() { } ] }` - filter, err := suite.postFilter(nil, nil, nil, nil, nil, nil, nil, &requestJson, http.StatusOK, "") + filter, err := suite.postFilter(nil, nil, nil, nil, nil, nil, nil, &requestJson, http.StatusOK, "", nil) if err != nil { suite.FailNow(err.Error()) } @@ -245,7 +247,7 @@ func (suite *FiltersTestSuite) TestPostFilterMinimal() { title := "GNU/Linux" context := []string{"home"} - filter, err := suite.postFilter(&title, &context, nil, nil, nil, nil, nil, nil, http.StatusOK, "") + filter, err := suite.postFilter(&title, &context, nil, nil, nil, nil, nil, nil, http.StatusOK, "", nil) if err != nil { suite.FailNow(err.Error()) } @@ -267,7 +269,7 @@ func (suite *FiltersTestSuite) TestPostFilterMinimal() { func (suite *FiltersTestSuite) TestPostFilterEmptyTitle() { title := "" context := []string{"home"} - _, err := suite.postFilter(&title, &context, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.postFilter(&title, &context, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "", nil) if err != nil { suite.FailNow(err.Error()) } @@ -275,7 +277,7 @@ func (suite *FiltersTestSuite) TestPostFilterEmptyTitle() { func (suite *FiltersTestSuite) TestPostFilterMissingTitle() { context := []string{"home"} - _, err := suite.postFilter(nil, &context, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.postFilter(nil, &context, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "", nil) if err != nil { suite.FailNow(err.Error()) } @@ -284,7 +286,7 @@ func (suite *FiltersTestSuite) TestPostFilterMissingTitle() { func (suite *FiltersTestSuite) TestPostFilterEmptyContext() { title := "GNU/Linux" context := []string{} - _, err := suite.postFilter(&title, &context, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.postFilter(&title, &context, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "", nil) if err != nil { suite.FailNow(err.Error()) } @@ -292,7 +294,7 @@ func (suite *FiltersTestSuite) TestPostFilterEmptyContext() { func (suite *FiltersTestSuite) TestPostFilterMissingContext() { title := "GNU/Linux" - _, err := suite.postFilter(&title, nil, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.postFilter(&title, nil, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "", nil) if err != nil { suite.FailNow(err.Error()) } @@ -301,8 +303,37 @@ func (suite *FiltersTestSuite) TestPostFilterMissingContext() { // Creating another filter with the same title should fail. func (suite *FiltersTestSuite) TestPostFilterTitleConflict() { title := suite.testFilters["local_account_1_filter_1"].Title - _, err := suite.postFilter(&title, nil, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "") + _, err := suite.postFilter(&title, nil, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, "", nil) if err != nil { suite.FailNow(err.Error()) } } + +// postFilterWithExpiration creates a filter with optional expiration. +func (suite *FiltersTestSuite) postFilterWithExpiration(title *string, expiresIn *int, expiresInStr *string, requestJson *string) *apimodel.FilterV2 { + context := []string{"home"} + filter, err := suite.postFilter(title, &context, nil, expiresIn, expiresInStr, nil, nil, requestJson, http.StatusOK, "", nil) + if err != nil { + suite.FailNow(err.Error()) + } + return filter +} + +// Regression test for https://github.com/superseriousbusiness/gotosocial/issues/3497 +func (suite *FiltersTestSuite) TestPostFilterWithEmptyStringExpiration() { + title := "Form Crimes" + expiresInStr := "" + filter := suite.postFilterWithExpiration(&title, nil, &expiresInStr, nil) + suite.Nil(filter.ExpiresAt) +} + +// Regression test related to https://github.com/superseriousbusiness/gotosocial/issues/3497 +func (suite *FiltersTestSuite) TestPostFilterWithNullExpirationJSON() { + requestJson := `{ + "title": "JSON Crimes", + "context": ["home"], + "expires_in": null + }` + filter := suite.postFilterWithExpiration(nil, nil, nil, &requestJson) + suite.Nil(filter.ExpiresAt) +} diff --git a/internal/api/client/filters/v2/filterput.go b/internal/api/client/filters/v2/filterput.go index cde03360d..9c1b56dd6 100644 --- a/internal/api/client/filters/v2/filterput.go +++ b/internal/api/client/filters/v2/filterput.go @@ -269,12 +269,11 @@ func validateNormalizeUpdateFilter(form *apimodel.FilterUpdateRequestV2) error { } } - // Normalize filter expiry if necessary. - if form.ExpiresInI != nil { - // If we parsed this as JSON, expires_in - // may be either a float64 or a string. + // If `expires_in` was provided + // as JSON, then normalize it. + if form.ExpiresInI.IsSpecified() { var err error - form.ExpiresIn, err = apiutil.ParseDuration( + form.ExpiresIn, err = apiutil.ParseNullableDuration( form.ExpiresInI, "expires_in", ) @@ -283,11 +282,6 @@ func validateNormalizeUpdateFilter(form *apimodel.FilterUpdateRequestV2) error { } } - // Interpret zero as indefinite duration. - if form.ExpiresIn != nil && *form.ExpiresIn == 0 { - form.ExpiresIn = nil - } - // Normalize and validate updates. for i, formKeyword := range form.Keywords { if formKeyword.Keyword != nil { diff --git a/internal/api/client/filters/v2/filterput_test.go b/internal/api/client/filters/v2/filterput_test.go index d82d84b20..afa858ba9 100644 --- a/internal/api/client/filters/v2/filterput_test.go +++ b/internal/api/client/filters/v2/filterput_test.go @@ -36,7 +36,7 @@ "github.com/superseriousbusiness/gotosocial/testrig" ) -func (suite *FiltersTestSuite) putFilter(filterID string, title *string, context *[]string, action *string, expiresIn *int, keywordsAttributesID *[]string, keywordsAttributesKeyword *[]string, keywordsAttributesWholeWord *[]bool, keywordsAttributesDestroy *[]bool, statusesAttributesID *[]string, statusesAttributesStatusID *[]string, statusesAttributesDestroy *[]bool, requestJson *string, expectedHTTPStatus int, expectedBody string) (*apimodel.FilterV2, error) { +func (suite *FiltersTestSuite) putFilter(filterID string, title *string, context *[]string, action *string, expiresIn *int, expiresInStr *string, keywordsAttributesKeyword *[]string, keywordsAttributesWholeWord *[]bool, keywordsAttributesDestroy *[]bool, statusesAttributesID *[]string, statusesAttributesStatusID *[]string, statusesAttributesDestroy *[]bool, requestJson *string, expectedHTTPStatus int, expectedBody string, keywordsAttributesID *[]string) (*apimodel.FilterV2, error) { // instantiate recorder + test context recorder := httptest.NewRecorder() ctx, _ := testrig.CreateGinTestContext(recorder, nil) @@ -64,6 +64,8 @@ func (suite *FiltersTestSuite) putFilter(filterID string, title *string, context } if expiresIn != nil { ctx.Request.Form["expires_in"] = []string{strconv.Itoa(*expiresIn)} + } else if expiresInStr != nil { + ctx.Request.Form["expires_in"] = []string{*expiresInStr} } if keywordsAttributesID != nil { ctx.Request.Form["keywords_attributes[][id]"] = *keywordsAttributesID @@ -159,7 +161,7 @@ func (suite *FiltersTestSuite) TestPutFilterFull() { keywordsAttributesWholeWord := []bool{true, false, true} keywordsAttributesDestroy := []bool{false, true} statusesAttributesStatusID := []string{suite.testStatuses["remote_account_1_status_2"].ID} - filter, err := suite.putFilter(id, &title, &context, &action, &expiresIn, &keywordsAttributesID, &keywordsAttributesKeyword, &keywordsAttributesWholeWord, &keywordsAttributesDestroy, nil, &statusesAttributesStatusID, nil, nil, http.StatusOK, "") + filter, err := suite.putFilter(id, &title, &context, &action, &expiresIn, nil, &keywordsAttributesKeyword, &keywordsAttributesWholeWord, &keywordsAttributesDestroy, nil, &statusesAttributesStatusID, nil, nil, http.StatusOK, "", &keywordsAttributesID) if err != nil { suite.FailNow(err.Error()) } @@ -231,7 +233,7 @@ func (suite *FiltersTestSuite) TestPutFilterFullJSON() { } ] }` - filter, err := suite.putFilter(id, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, &requestJson, http.StatusOK, "") + filter, err := suite.putFilter(id, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, &requestJson, http.StatusOK, "", nil) if err != nil { suite.FailNow(err.Error()) } @@ -281,7 +283,7 @@ func (suite *FiltersTestSuite) TestPutFilterMinimal() { id := suite.testFilters["local_account_1_filter_1"].ID title := "GNU/Linux" context := []string{"home"} - filter, err := suite.putFilter(id, &title, &context, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusOK, "") + filter, err := suite.putFilter(id, &title, &context, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusOK, "", nil) if err != nil { suite.FailNow(err.Error()) } @@ -302,7 +304,7 @@ func (suite *FiltersTestSuite) TestPutFilterEmptyTitle() { id := suite.testFilters["local_account_1_filter_1"].ID title := "" context := []string{"home"} - _, err := suite.putFilter(id, &title, &context, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, `{"error":"Unprocessable Entity: filter title must be provided, and must be no more than 200 chars"}`) + _, err := suite.putFilter(id, &title, &context, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, `{"error":"Unprocessable Entity: filter title must be provided, and must be no more than 200 chars"}`, nil) if err != nil { suite.FailNow(err.Error()) } @@ -312,7 +314,7 @@ func (suite *FiltersTestSuite) TestPutFilterEmptyContext() { id := suite.testFilters["local_account_1_filter_1"].ID title := "GNU/Linux" context := []string{} - _, err := suite.putFilter(id, &title, &context, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, `{"error":"Unprocessable Entity: at least one filter context is required"}`) + _, err := suite.putFilter(id, &title, &context, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusUnprocessableEntity, `{"error":"Unprocessable Entity: at least one filter context is required"}`, nil) if err != nil { suite.FailNow(err.Error()) } @@ -322,7 +324,7 @@ func (suite *FiltersTestSuite) TestPutFilterEmptyContext() { func (suite *FiltersTestSuite) TestPutFilterTitleConflict() { id := suite.testFilters["local_account_1_filter_1"].ID title := suite.testFilters["local_account_1_filter_2"].Title - _, err := suite.putFilter(id, &title, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusConflict, `{"error":"Conflict: you already have a filter with this title"}`) + _, err := suite.putFilter(id, &title, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusConflict, `{"error":"Conflict: you already have a filter with this title"}`, nil) if err != nil { suite.FailNow(err.Error()) } @@ -332,7 +334,7 @@ func (suite *FiltersTestSuite) TestPutAnotherAccountsFilter() { id := suite.testFilters["local_account_2_filter_1"].ID title := "GNU/Linux" context := []string{"home"} - _, err := suite.putFilter(id, &title, &context, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusNotFound, `{"error":"Not Found"}`) + _, err := suite.putFilter(id, &title, &context, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusNotFound, `{"error":"Not Found"}`, nil) if err != nil { suite.FailNow(err.Error()) } @@ -342,8 +344,70 @@ func (suite *FiltersTestSuite) TestPutNonexistentFilter() { id := "not_even_a_real_ULID" phrase := "GNU/Linux" context := []string{"home"} - _, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusNotFound, `{"error":"Not Found"}`) + _, err := suite.putFilter(id, &phrase, &context, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, http.StatusNotFound, `{"error":"Not Found"}`, nil) if err != nil { suite.FailNow(err.Error()) } } + +// setFilterExpiration sets filter expiration. +func (suite *FiltersTestSuite) setFilterExpiration(id string, expiresIn *int, expiresInStr *string, requestJson *string) *apimodel.FilterV2 { + filter, err := suite.putFilter(id, nil, nil, nil, expiresIn, expiresInStr, nil, nil, nil, nil, nil, nil, requestJson, http.StatusOK, "", nil) + if err != nil { + suite.FailNow(err.Error()) + } + return filter +} + +// Regression test for https://github.com/superseriousbusiness/gotosocial/issues/3497 +func (suite *FiltersTestSuite) TestPutFilterUnsetExpirationDateEmptyString() { + id := suite.testFilters["local_account_1_filter_2"].ID + + // Setup: set an expiration date for the filter. + expiresIn := 86400 + filter := suite.setFilterExpiration(id, &expiresIn, nil, nil) + if !suite.NotNil(filter.ExpiresAt) { + suite.FailNow("Test precondition failed") + } + + // Unset the filter's expiration date by setting it to an empty string. + expiresInStr := "" + filter = suite.setFilterExpiration(id, nil, &expiresInStr, nil) + suite.Nil(filter.ExpiresAt) +} + +// Regression test related to https://github.com/superseriousbusiness/gotosocial/issues/3497 +func (suite *FiltersTestSuite) TestPutFilterUnsetExpirationDateNullJSON() { + id := suite.testFilters["local_account_1_filter_3"].ID + + // Setup: set an expiration date for the filter. + expiresIn := 86400 + filter := suite.setFilterExpiration(id, &expiresIn, nil, nil) + if !suite.NotNil(filter.ExpiresAt) { + suite.FailNow("Test precondition failed") + } + + // Unset the filter's expiration date by setting it to a null literal. + requestJson := `{ + "expires_in": null + }` + filter = suite.setFilterExpiration(id, nil, nil, &requestJson) + suite.Nil(filter.ExpiresAt) +} + +// Regression test related to https://github.com/superseriousbusiness/gotosocial/issues/3497 +func (suite *FiltersTestSuite) TestPutFilterUnalteredExpirationDateJSON() { + id := suite.testFilters["local_account_1_filter_4"].ID + + // Setup: set an expiration date for the filter. + expiresIn := 86400 + filter := suite.setFilterExpiration(id, &expiresIn, nil, nil) + if !suite.NotNil(filter.ExpiresAt) { + suite.FailNow("Test precondition failed") + } + + // Update nothing. There should still be an expiration date. + requestJson := `{}` + filter = suite.setFilterExpiration(id, nil, nil, &requestJson) + suite.NotNil(filter.ExpiresAt) +} diff --git a/internal/api/model/filterv1.go b/internal/api/model/filterv1.go index 1c3b5fb8e..0b092627e 100644 --- a/internal/api/model/filterv1.go +++ b/internal/api/model/filterv1.go @@ -95,5 +95,5 @@ type FilterCreateUpdateRequestV1 struct { // Number of seconds from now that the filter should expire. If omitted, filter never expires. // // Example: 86400 - ExpiresInI interface{} `json:"expires_in"` + ExpiresInI Nullable[any] `json:"expires_in"` } diff --git a/internal/api/model/filterv2.go b/internal/api/model/filterv2.go index 242c569dc..26b1b22b3 100644 --- a/internal/api/model/filterv2.go +++ b/internal/api/model/filterv2.go @@ -134,7 +134,7 @@ type FilterCreateRequestV2 struct { // Number of seconds from now that the filter should expire. If omitted, filter never expires. // // Example: 86400 - ExpiresInI interface{} `json:"expires_in"` + ExpiresInI Nullable[any] `json:"expires_in"` // Keywords to be added to the newly created filter. Keywords []FilterKeywordCreateUpdateRequest `form:"-" json:"keywords_attributes" xml:"keywords_attributes"` @@ -199,7 +199,7 @@ type FilterUpdateRequestV2 struct { // Number of seconds from now that the filter should expire. If omitted, filter never expires. // // Example: 86400 - ExpiresInI interface{} `json:"expires_in"` + ExpiresInI Nullable[any] `json:"expires_in"` // Keywords to be added to the filter, modified, or removed. Keywords []FilterKeywordCreateUpdateDeleteRequest `form:"-" json:"keywords_attributes" xml:"keywords_attributes"` diff --git a/internal/api/model/nullable.go b/internal/api/model/nullable.go new file mode 100644 index 000000000..4dd02f854 --- /dev/null +++ b/internal/api/model/nullable.go @@ -0,0 +1,107 @@ +// GoToSocial +// Copyright (C) GoToSocial Authors admin@gotosocial.org +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package model + +import ( + "bytes" + "encoding/json" + "errors" +) + +// Nullable is a generic type, which implements a field that can be one of three states: +// +// - field is not set in the request +// - field is explicitly set to `null` in the request +// - field is explicitly set to a valid value in the request +// +// Nullable is intended to be used with JSON unmarshalling. +// +// Adapted from https://github.com/oapi-codegen/nullable/blob/main/nullable.go +type Nullable[T any] struct { + state nullableState + value T +} + +type nullableState uint8 + +const ( + nullableStateUnspecified nullableState = 0 + nullableStateNull nullableState = 1 + nullableStateSet nullableState = 2 +) + +// Get retrieves the underlying value, if present, +// and returns an error if the value was not present. +func (t Nullable[T]) Get() (T, error) { + var empty T + if t.IsNull() { + return empty, errors.New("value is null") + } + + if !t.IsSpecified() { + return empty, errors.New("value is not specified") + } + + return t.value, nil +} + +// IsNull indicates whether the field +// was sent, and had a value of `null` +func (t Nullable[T]) IsNull() bool { + return t.state == nullableStateNull +} + +// IsSpecified indicates whether the field +// was sent either as a value or as `null`. +func (t Nullable[T]) IsSpecified() bool { + return t.state != nullableStateUnspecified +} + +// If field is unspecified, +// UnmarshalJSON won't be called. +func (t *Nullable[T]) UnmarshalJSON(data []byte) error { + // If field is specified as `null`. + if bytes.Equal(data, []byte("null")) { + t.setNull() + return nil + } + + // Otherwise, we have an + // actual value, so parse it. + var v T + if err := json.Unmarshal(data, &v); err != nil { + return err + } + + t.set(v) + return nil +} + +// setNull indicates that the field +// was sent, and had a value of `null` +func (t *Nullable[T]) setNull() { + *t = Nullable[T]{state: nullableStateNull} +} + +// set the underlying value to given value. +func (t *Nullable[T]) set(value T) { + *t = Nullable[T]{ + state: nullableStateSet, + value: value, + } +} diff --git a/internal/api/util/parseform.go b/internal/api/util/parseform.go index 19e24189f..3eab065f2 100644 --- a/internal/api/util/parseform.go +++ b/internal/api/util/parseform.go @@ -20,12 +20,13 @@ import ( "fmt" "strconv" + + apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" + "github.com/superseriousbusiness/gotosocial/internal/util" ) -// ParseDuration parses the given raw interface belonging to +// ParseDuration parses the given raw interface belonging // the given fieldName as an integer duration. -// -// Will return nil, nil if rawI is the zero value of its type. func ParseDuration(rawI any, fieldName string) (*int, error) { var ( asInteger int @@ -60,11 +61,28 @@ func ParseDuration(rawI any, fieldName string) (*int, error) { return nil, err } - // Someone submitted 0, - // don't point to this. - if asInteger == 0 { - return nil, nil - } - return &asInteger, nil } + +// ParseNullableDuration is like ParseDuration, but +// for JSON values that may have been sent as `null`. +// +// IsSpecified should be checked and "true" on the +// given nullable before calling this function. +func ParseNullableDuration( + nullable apimodel.Nullable[any], + fieldName string, +) (*int, error) { + if nullable.IsNull() { + // Was specified as `null`, + // return pointer to zero value. + return util.Ptr(0), nil + } + + rawI, err := nullable.Get() + if err != nil { + return nil, err + } + + return ParseDuration(rawI, fieldName) +} diff --git a/internal/processing/filters/v1/create.go b/internal/processing/filters/v1/create.go index 18367dfce..86019d2a6 100644 --- a/internal/processing/filters/v1/create.go +++ b/internal/processing/filters/v1/create.go @@ -43,7 +43,7 @@ func (p *Processor) Create(ctx context.Context, account *gtsmodel.Account, form if *form.Irreversible { filter.Action = gtsmodel.FilterActionHide } - if form.ExpiresIn != nil { + if form.ExpiresIn != nil && *form.ExpiresIn != 0 { filter.ExpiresAt = time.Now().Add(time.Second * time.Duration(*form.ExpiresIn)) } for _, context := range form.Context { diff --git a/internal/processing/filters/v1/update.go b/internal/processing/filters/v1/update.go index 81340b4be..15c5de365 100644 --- a/internal/processing/filters/v1/update.go +++ b/internal/processing/filters/v1/update.go @@ -67,7 +67,7 @@ func (p *Processor) Update( action = gtsmodel.FilterActionHide } expiresAt := time.Time{} - if form.ExpiresIn != nil { + if form.ExpiresIn != nil && *form.ExpiresIn != 0 { expiresAt = time.Now().Add(time.Second * time.Duration(*form.ExpiresIn)) } contextHome := false diff --git a/internal/processing/filters/v2/create.go b/internal/processing/filters/v2/create.go index 7095a643c..60dd46f43 100644 --- a/internal/processing/filters/v2/create.go +++ b/internal/processing/filters/v2/create.go @@ -41,7 +41,7 @@ func (p *Processor) Create(ctx context.Context, account *gtsmodel.Account, form Title: form.Title, Action: typeutils.APIFilterActionToFilterAction(*form.FilterAction), } - if form.ExpiresIn != nil { + if form.ExpiresIn != nil && *form.ExpiresIn != 0 { filter.ExpiresAt = time.Now().Add(time.Second * time.Duration(*form.ExpiresIn)) } for _, context := range form.Context { diff --git a/internal/processing/filters/v2/update.go b/internal/processing/filters/v2/update.go index 0d443d58e..d5b5cce01 100644 --- a/internal/processing/filters/v2/update.go +++ b/internal/processing/filters/v2/update.go @@ -21,8 +21,6 @@ "context" "errors" "fmt" - "time" - apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtserror" @@ -30,6 +28,7 @@ "github.com/superseriousbusiness/gotosocial/internal/id" "github.com/superseriousbusiness/gotosocial/internal/typeutils" "github.com/superseriousbusiness/gotosocial/internal/util" + "time" ) // Update an existing filter for the given account, using the provided parameters. @@ -68,10 +67,16 @@ func (p *Processor) Update( filterColumns = append(filterColumns, "action") filter.Action = typeutils.APIFilterActionToFilterAction(*form.FilterAction) } - // TODO: (Vyr) is it possible to unset a filter expiration with this API? if form.ExpiresIn != nil { + expiresIn := *form.ExpiresIn filterColumns = append(filterColumns, "expires_at") - filter.ExpiresAt = time.Now().Add(time.Second * time.Duration(*form.ExpiresIn)) + if expiresIn == 0 { + // Unset the expiration date. + filter.ExpiresAt = time.Time{} + } else { + // Update the expiration date. + filter.ExpiresAt = time.Now().Add(time.Second * time.Duration(expiresIn)) + } } if form.Context != nil { filterColumns = append(filterColumns,