Skip to content

Commit 00b0a31

Browse files
authored
[TT-14370] [OAS] ReadableDuration converts some values to decimals causing a schema problem (#7256)
### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-14370" title="TT-14370" target="_blank">TT-14370</a></summary> <br /> <table> <tr> <th>Summary</th> <td>[OAS] ReadableDuration converts some values to decimals causing a schema problem</td> </tr> <tr> <th>Type</th> <td> <img alt="Bug" src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium" /> Bug </td> </tr> <tr> <th>Status</th> <td>In Dev</td> </tr> <tr> <th>Points</th> <td>N/A</td> </tr> <tr> <th>Labels</th> <td><a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20codilime_refined%20ORDER%20BY%20created%20DESC" title="codilime_refined">codilime_refined</a></td> </tr> </table> </details> <!-- do not remove this marker as it will break jira-lint's functionality. added_by_jira_lint --> --- <!-- Provide a general summary of your changes in the Title above --> ## Description <!-- Describe your changes in detail --> ## Related Issue <!-- This project only accepts pull requests related to open issues. --> <!-- If suggesting a new feature or change, please discuss it in an issue first. --> <!-- If fixing a bug, there should be an issue describing it with steps to reproduce. --> <!-- OSS: Please link to the issue here. Tyk: please create/link the JIRA ticket. --> ## Motivation and Context <!-- Why is this change required? What problem does it solve? --> ## How This Has Been Tested <!-- Please describe in detail how you tested your changes --> <!-- Include details of your testing environment, and the tests --> <!-- you ran to see how your change affects other areas of the code, etc. --> <!-- This information is helpful for reviewers and QA. --> ## Screenshots (if appropriate) ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Refactoring or add test (improvements in base code or adds test coverage to functionality) ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply --> <!-- If there are no documentation updates required, mark the item as checked. --> <!-- Raise up any additional concerns not covered by the checklist. --> - [ ] I ensured that the documentation is up to date - [ ] I explained why this PR updates go.mod in detail with reasoning why it's required - [ ] I would like a code coverage CI quality gate exception and have explained why ___ ### **PR Type** Bug fix, Tests ___ ### **Description** - Fixes duration serialization to avoid decimal values in JSON. - Implements custom formatting for `ReadableDuration` output. - Adds new test cases for composite durations (e.g., "1m30s"). - Updates expected JSON outputs in tests for consistency. ___ ### Diagram Walkthrough ```mermaid flowchart LR durationGo["duration.go: Add custom format for ReadableDuration"] -- "uses" --> durationTestGo["duration_test.go: Update & add tests for new format"] durationGo -- "fixes" --> "JSON serialization issue" durationTestGo -- "validates" --> "Correct output for various durations" ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>duration.go</strong><dd><code>Custom formatting for ReadableDuration JSON serialization</code></dd></summary> <hr> internal/time/duration.go <ul><li>Adds a custom <code>format()</code> method for <code>ReadableDuration</code>.<br> <li> Ensures JSON serialization outputs only integer values and valid <br>patterns.<br> <li> Introduces constants and a conversion table for time units.<br> <li> Refactors <code>MarshalJSON</code> to use the new formatting logic.</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7256/files#diff-6e8ef3118f84cbcc935f27d5a3ad5f4eb86eb22728400e9322c9b796b9d8d855">+53/-1</a>&nbsp; &nbsp; </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>duration_test.go</strong><dd><code>Update and expand tests for duration serialization</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> internal/time/duration_test.go <ul><li>Updates expected JSON output for minute-based durations.<br> <li> Adds new test for composite durations (e.g., "1m30s").<br> <li> Ensures tests align with new serialization logic.</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7256/files#diff-71942cdc77128266498b62e712f82d0c63bbb39d236fe9e6677f49080c28cea1">+9/-1</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></td></tr></tr></tbody></table> </details> ___
1 parent 5460a49 commit 00b0a31

File tree

2 files changed

+80
-2
lines changed

2 files changed

+80
-2
lines changed

internal/time/duration.go

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"math"
77
"strconv"
8+
"strings"
89
"time"
910
)
1011

@@ -26,6 +27,15 @@ const (
2627
Hour = time.Hour
2728
)
2829

30+
const (
31+
EndingHour = "h"
32+
EndingMinute = "m"
33+
EndingSecond = "s"
34+
EndingMillisecond = "ms"
35+
EndingMicrosecond = "µs"
36+
EndingNanosecond = "ns"
37+
)
38+
2939
// ReadableDuration is a type alias for time.Duration, so that shorthand notation can be used.
3040
// Examples of valid shorthand notations:
3141
// - "1h" : one hour
@@ -41,7 +51,7 @@ type ReadableDuration time.Duration
4151

4252
// MarshalJSON converts ReadableDuration into human-readable shorthand notation for time.Duration into json format.
4353
func (d ReadableDuration) MarshalJSON() ([]byte, error) {
44-
return []byte(fmt.Sprintf(`"%s"`, time.Duration(d).String())), nil
54+
return []byte(fmt.Sprintf(`"%s"`, d.format())), nil
4555
}
4656

4757
// UnmarshalJSON converts human-readable shorthand notation for time.Duration into ReadableDuration from json format.
@@ -82,3 +92,50 @@ func (d ReadableDuration) Nanoseconds() int64 {
8292
func (d ReadableDuration) Microseconds() int64 {
8393
return Duration(d).Microseconds()
8494
}
95+
96+
// formats time duration dur to validation pattern ^(\d+h)?(\d+m)?(\d+s)?(\d+ms)?(\d+µs)?(\d+ns)%
97+
func (d ReadableDuration) format() string {
98+
if d == 0 {
99+
return "0s"
100+
}
101+
102+
var sb strings.Builder
103+
var rest = int64(d)
104+
105+
if rest < 0 {
106+
rest *= -1
107+
sb.WriteByte('-')
108+
}
109+
110+
for _, conv := range convertCases {
111+
if rest == 0 {
112+
break
113+
}
114+
115+
curr := rest / int64(conv.duration)
116+
rest -= curr * int64(conv.duration)
117+
118+
if curr == 0 {
119+
continue
120+
}
121+
122+
sb.WriteString(strconv.FormatInt(curr, 10))
123+
sb.WriteString(conv.ending)
124+
}
125+
126+
return sb.String()
127+
}
128+
129+
type convertCase struct {
130+
duration time.Duration
131+
ending string
132+
}
133+
134+
var convertCases = []convertCase{
135+
{time.Hour, EndingHour},
136+
{time.Minute, EndingMinute},
137+
{time.Second, EndingSecond},
138+
{time.Millisecond, EndingMillisecond},
139+
{time.Microsecond, EndingMicrosecond},
140+
{time.Nanosecond, EndingNanosecond},
141+
}

internal/time/duration_test.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ func TestReadableDuration_MarshalJSON(t *testing.T) {
1313

1414
t.Run("valid", func(t *testing.T) {
1515
duration := ReadableDuration(time.Minute * 5)
16-
expectedJSON := []byte(`"5m0s"`)
16+
expectedJSON := []byte(`"5m"`)
1717
resultJSON, err := json.Marshal(&duration)
1818
assert.NoError(t, err)
1919
assert.Equal(t, string(expectedJSON), string(resultJSON))
@@ -34,6 +34,27 @@ func TestReadableDuration_MarshalJSON(t *testing.T) {
3434
assert.NoError(t, err)
3535
assert.Equal(t, string(expectedJSON), string(resultJSON))
3636
})
37+
38+
t.Run("90 seconds", func(t *testing.T) {
39+
duration := ReadableDuration(time.Second * 90)
40+
expectedJSON := []byte(`"1m30s"`)
41+
resultJSON, err := json.Marshal(&duration)
42+
assert.NoError(t, err)
43+
assert.Equal(t, string(expectedJSON), string(resultJSON))
44+
})
45+
46+
t.Run("complex duration", func(t *testing.T) {
47+
duration := ReadableDuration(1*time.Hour + 2*time.Minute + 3*time.Second + 4*time.Millisecond + 5*time.Microsecond + 6*time.Nanosecond)
48+
assert.Equal(t, "1h2m3s4ms5µs6ns", duration.format())
49+
50+
duration = ReadableDuration(1*time.Hour + (2+60)*time.Minute + 3*time.Second + 4*time.Millisecond + 5*time.Microsecond + 6*time.Nanosecond)
51+
assert.Equal(t, "2h2m3s4ms5µs6ns", duration.format())
52+
})
53+
54+
t.Run("negative duration", func(t *testing.T) {
55+
duration := ReadableDuration(-1 * time.Second * 90)
56+
assert.Equal(t, "-1m30s", duration.format())
57+
})
3758
}
3859

3960
func TestReadableDuration_Seconds(t *testing.T) {

0 commit comments

Comments
 (0)