Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-44052

Incorrect usage of TimeDuration

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Minor
    • Resolution: Fixed
    • Component/s: core
    • Labels:
    • Similar Issues:

      Description

      When Kohsuke Kawaguchi introduced TimeDuration in 2012, there were mistakes in the handling of time units. The naming of the internal field millis implies that it is measured in milliseconds, as does the design of the getTimeInMillis and as methods. Yet fromString treats 3sec as new TimeDuration(3), treating the field as measured in seconds.

      Why did we not notice wildly incorrect scheduling times? Because the callers of this utility, currently ParametersDefinitionProperty and ParameterizedJobMixIn, also use it incorrectly. If the query parameter is set, they ask it for getTime (supposedly in milliseconds), yet pass it to scheduling methods which expect to be measured in seconds (as do quietPeriod fields). If it is not set, they construct it from quiet periods, passing in a value measured in seconds.

      The parsing code was also intended to support other units, but never did.

      The effect of all this mess is that .../build, .../build?delay=3sec, and .../build?delay=3 all behave as you expect, so there is not really a user-visible bug, but the code is very confusing.

      Since TimeDuration is not Serializable, there is no stored-settings issue here, so it would suffice to change <init>(long) to * 1000 for compatibility but deprecate it in favor of a constructor taking an explicit unit; change getTime to / 1000 and deprecate in favor of as; and change all call sites to use nondeprecated forms, passing in TimeUnit.SECONDS.

        Attachments

          Issue Links

            Activity

            jglick Jesse Glick created issue -
            batmat Baptiste Mathus made changes -
            Field Original Value New Value
            Assignee Baptiste Mathus [ batmat ]
            batmat Baptiste Mathus made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            batmat Baptiste Mathus made changes -
            Status In Progress [ 3 ] In Review [ 10005 ]
            batmat Baptiste Mathus made changes -
            Remote Link Cette demande est liée à "Core PR-3043 (Lien Web)" [ 17776 ]
            danielbeck Daniel Beck made changes -
            Resolution Fixed [ 1 ]
            Status In Review [ 10005 ] Resolved [ 5 ]
            tpatteri Tapio Patteri made changes -
            Link This issue relates to JENKINS-48770 [ JENKINS-48770 ]
            jglick Jesse Glick made changes -
            Link This issue causes JENKINS-56542 [ JENKINS-56542 ]

              People

              Assignee:
              batmat Baptiste Mathus
              Reporter:
              jglick Jesse Glick
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: