Skip to content

Conversation

@lindseysimple
Copy link
Contributor

@lindseysimple lindseysimple commented Aug 8, 2025

Resolves #1013. Support day unit in edgex-dto-duration validation tag.

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/go-mod-core-contracts/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?)
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?)
  • I have opened a PR for the related docs change (if not, why?)

Testing Instructions

New Dependency Instructions (If applicable)

Resolves edgexfoundry#1013. Support day unit in edgex-dto-duration validation tag.

Signed-off-by: Lindsey Cheng <[email protected]>
@lindseysimple lindseysimple changed the title feat: Support day unit in edgex-dto-duration validation tag feat: Support day unit with edgex-dto-duration validation tag Aug 8, 2025
@lindseysimple lindseysimple requested a review from judehung August 8, 2025 09:02
Comment on lines 183 to 218
func parseDurationWithDay(durationStr string) (bool, time.Duration) {
var totalDuration time.Duration

// Regex to capture decimal days (e.g., "2.5d" or "3d")
re := regexp.MustCompile(`([\d.]+)d`)
matches := re.FindStringSubmatch(durationStr)

// Checks if the duration string contains a valid decimal with 'd'(day) unit
if len(matches) == 2 {
day, err := strconv.ParseFloat(matches[1], 64)
if err != nil {
return false, 0
}
// Converts days to hours
totalDuration = time.Duration(day * float64(24*time.Hour))

// Remove the matched "Xd" (e.g., "2.5d") from the duration string
// so we're left with only the remaining duration (e.g., "5h")
durationStr = re.ReplaceAllString(durationStr, "")
durationStr = strings.TrimSpace(durationStr)

// The duration string contains no other time units except for "d"
if durationStr == "" {
return true, totalDuration
}
}

// Parse the remaining duration string without day unit
remainingDuration, err := time.ParseDuration(durationStr)
if err != nil {
return false, totalDuration
}

totalDuration += remainingDuration
return true, totalDuration
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The time.ParseDuration is capable of summing up repeated time units inside the duration string. For example, time.ParseDuration("1h30m10m1h") will return a duration 2h40m0s. To be consistent with such behavior, the implementation of parseDurationWithDay should be capable of doing same thing to sum up repeated d time unit. Below is a revised version of parseDurationWithDay to achieve this:

func parseDurationWithDay(durationStr string) (bool, time.Duration) {
	var totalDuration time.Duration

	// Regex to find all day fragments like "2.2d", "1.5d", etc.
	re := regexp.MustCompile(`([\d.]+)d`)
	matches := re.FindAllStringSubmatch(durationStr, -1)

	// Sum up all matched day durations
	for _, match := range matches {
		if len(match) != 2 {
			continue
		}
		dayVal, err := strconv.ParseFloat(match[1], 64)
		if err != nil {
			return false, 0
		}
		totalDuration += time.Duration(dayVal * float64(24*time.Hour))
	}

	// Remove all day fragments from the original string to get the rest
	durationStr = re.ReplaceAllString(durationStr, "")
	durationStr = strings.TrimSpace(durationStr)

	// Parse the remaining standard duration if any (e.g., "5h30m")
	if durationStr != "" {
		remainingDuration, err := time.ParseDuration(durationStr)
		if err != nil {
			return false, totalDuration
		}
		totalDuration += remainingDuration
	}

	return true, totalDuration
}

We should also unit tests to cover such edge case, e.g. 2.2d1.1d20m3h1h10m is parsed to 83h42m0s.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out the repeated day units case which I didn't think of.
Modified and unit test added.

Handle repeated days in duration string.

Signed-off-by: Lindsey Cheng <[email protected]>
@lindseysimple lindseysimple requested a review from judehung August 11, 2025 01:50
Copy link
Member

@judehung judehung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@judehung judehung merged commit 11bffbc into edgexfoundry:main Aug 11, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support day unit in ValidateDuration validation function

2 participants