Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Timestamps should use currentTimeMillis #822

Merged
merged 1 commit into from Jul 10, 2019
Merged

Timestamps should use currentTimeMillis #822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion metals/src/main/scala/scala/meta/internal/builds/Digests.scala
View file
Open in desktop
Expand Up @@ -15,7 +15,7 @@ final class Digests(conn: () => Connection, time: Time) {
conn().update(
s"insert into sbt_digest values (?, ?, ?);"
) { stmt =>
val timestamp = new Timestamp(time.millis())
val timestamp = new Timestamp(time.currentMillis())
stmt.setString(1, md5Digest)
stmt.setByte(2, status.value.toByte)
stmt.setTimestamp(3, timestamp)
Expand Down
2 changes: 1 addition & 1 deletion metals/src/main/scala/scala/meta/internal/metals/ChosenBuildServers.scala
View file
Open in desktop
Expand Up @@ -21,7 +21,7 @@ class ChosenBuildServers(conn: () => Connection, time: Time) {
conn().update(
s"merge into chosen_build_server key(md5) values (?, ?, ?);"
) { stmt =>
val timestamp = new Timestamp(time.millis())
val timestamp = new Timestamp(time.currentMillis())
stmt.setString(1, md5)
stmt.setString(2, server)
stmt.setTimestamp(3, timestamp)
Expand Down
1 change: 1 addition & 0 deletions metals/src/main/scala/scala/meta/internal/metals/Configs.scala
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@ object Configs {
new DidChangeWatchedFilesRegistrationOptions(
List(
new FileSystemWatcher(s"$root/*.sbt"),
new FileSystemWatcher(s"$root/pom.xml"),
new FileSystemWatcher(s"$root/*.sc"),
new FileSystemWatcher(s"$root/*?.gradle"),
new FileSystemWatcher(s"$root/*.gradle.kts"),
Expand Down
6 changes: 3 additions & 3 deletions metals/src/main/scala/scala/meta/internal/metals/DismissedNotifications.scala
View file
Open in desktop
Expand Up @@ -15,7 +15,7 @@ final class DismissedNotifications(conn: () => Connection, time: Time) {
class Notification(val id: Int)(implicit name: sourcecode.Name) {
override def toString: String = s"Notification(${name.value}, $id)"
def isDismissed: Boolean = {
val now = new Timestamp(time.millis())
val now = new Timestamp(time.currentMillis())
conn().query {
"select * from dismissed_notification where id = ? and when_expires > ? limit 1;"
} { stmt =>
Expand All @@ -29,12 +29,12 @@ final class DismissedNotifications(conn: () => Connection, time: Time) {
dismiss(10000, TimeUnit.DAYS)
}
def dismiss(count: Long, unit: TimeUnit): Unit = {
val sum = time.millis() + unit.toMillis(count)
val sum = time.currentMillis() + unit.toMillis(count)
if (sum < 0) dismissForever()
else dismiss(new Timestamp(sum))
}
def dismiss(whenExpire: Timestamp): Unit = {
val now = new Timestamp(time.millis())
val now = new Timestamp(time.currentMillis())
conn().update {
"insert into dismissed_notification values (?, ?, ?);"
} { stmt =>
Expand Down
8 changes: 4 additions & 4 deletions metals/src/main/scala/scala/meta/internal/metals/Time.scala
View file
Open in desktop
@@ -1,17 +1,17 @@
package scala.meta.internal.metals

import java.util.concurrent.TimeUnit

/**
* Wrapper around `System.nanoTime` to allow easier testing.
* Wrapper around `System.nanoTime` and `System.currentTimeMillis`
* to allow easier testing.
*/
trait Time {
def nanos(): Long
final def millis(): Long = TimeUnit.NANOSECONDS.toMillis(nanos())
def currentMillis(): Long
}

object Time {
object system extends Time {
def nanos(): Long = System.nanoTime()
Copy link
Contributor

Choose a reason for hiding this comment

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

We can also include the "absolute time base" for proper calculation of the elapsed nanoseconds:

Suggested change
def nanos(): Long = System.nanoTime()
private val absoluteTimeBaseNanos: Long = {
val now = Instant.now()
val seconds = TimeUnit.SECONDS.toNanos(now.getEpochSecond)
val nanos = now.getNano
seconds + nanos
}
private val relativeTimeBaseNanos = System.nanoTime()
def nanos(): Long = {
absoluteTimeBaseNanos + elapsed
}
private def elapsed: Long = System.nanoTime() - relativeTimeBaseNanos

Note, that this does not handle the time zones at all. Probably it is safe to assume the time of the server as the valid one. At least for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it might be too much complexity for now.

def currentMillis(): Long = System.currentTimeMillis()
}
}
2 changes: 2 additions & 0 deletions tests/unit/src/main/scala/tests/FakeTime.scala
View file
Open in desktop
Expand Up @@ -13,4 +13,6 @@ class FakeTime extends Time {
elapse(n, TimeUnit.SECONDS)
}
override def nanos(): Long = elapsed

override def currentMillis(): Long = TimeUnit.NANOSECONDS.toMillis(elapsed)
}
4 changes: 2 additions & 2 deletions tests/unit/src/test/scala/tests/DigestsSuite.scala
View file
Open in desktop
Expand Up @@ -9,14 +9,14 @@ object DigestsSuite extends BaseTablesSuite {
assertEquals(digests.setStatus("a", Requested), 1)
assertEquals(
digests.last().get,
Digest("a", Requested, time.millis())
Digest("a", Requested, time.currentMillis())
)
time.elapseSeconds(1)
assertEquals(digests.getStatus("a").get, Requested)
assertEquals(digests.setStatus("a", Installed), 1)
assertEquals(
digests.last().get,
Digest("a", Installed, time.millis())
Digest("a", Installed, time.currentMillis())
)
time.elapseSeconds(1)
assertEquals(digests.getStatus("a").get, Installed)
Expand Down