opendev/gerrit
Code Issues Proposed changes

Use TimeUnits for sort key calculations

Browse Source 
The unspecified units were confusing when writing tests around these
values, so be explicit about units in variable names and quantities
for conversion.

Use Ints.checkedCast so we fail fast when inserting a change 4000+
years in the future rather than having it not show up in search
results.

Correct the sort key epoch time, which likely had a timezone error
when initially calculated:

$ python -c 'import datetime; print datetime.datetime.fromtimestamp(1222819200L)'
2008-09-30 17:00:00

This mistake does not affect any data; the comment in the code was
just confusingly incorrect.

Change-Id: I6d8bcf4985e91c3d115002db42642049da8c708f
This commit is contained in:
Dave Borowitz
2013-10-15 10:05:59 -07:00
parent ee7fbf3a3b
commit a2023e9486
2 changed files with 27 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
View File

@@ -15,8 +15,12 @@
package com.google.gerrit.server; package com.google.gerrit.server;
import static com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy.RECEIVE_COMMITS; import static com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy.RECEIVE_COMMITS;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
@@ -80,8 +84,14 @@ import java.util.Set;
import java.util.regex.Matcher; import java.util.regex.Matcher;
public class ChangeUtil { public class ChangeUtil {
/**
* Epoch for sort key calculations, Tue Sep 30 2008 17:00:00.
* <p>
* We overrun approximately 4,083 years later, so ~6092.
*/
@VisibleForTesting @VisibleForTesting
public static final long SORT_KEY_EPOCH = 1222819200L; // Oct 1 2008 00:00 public static final long SORT_KEY_EPOCH_MINS =
MINUTES.convert(1222819200L, SECONDS);
private static final Object uuidLock = new Object(); private static final Object uuidLock = new Object();
private static final int SEED = 0x2418e6f9; private static final int SEED = 0x2418e6f9;
@@ -465,14 +475,12 @@ public class ChangeUtil {
db.patchSets().delete(Collections.singleton(patch)); db.patchSets().delete(Collections.singleton(patch));
} }
public static String sortKey(long lastUpdated, int id){ public static String sortKey(long lastUpdatedMs, int id){
// The encoding uses minutes since Wed Oct 1 00:00:00 2008 UTC. long lastUpdatedMins = MINUTES.convert(lastUpdatedMs, MILLISECONDS);
// We overrun approximately 4,085 years later, so ~6093. long minsSinceEpoch = lastUpdatedMins - SORT_KEY_EPOCH_MINS;
// StringBuilder r = new StringBuilder(16);
final long lastUpdatedOn = (lastUpdated / 1000L) - SORT_KEY_EPOCH;
final StringBuilder r = new StringBuilder(16);
r.setLength(16); r.setLength(16);
formatHexInt(r, 0, (int) (lastUpdatedOn / 60)); formatHexInt(r, 0, Ints.checkedCast(minsSinceEpoch));
formatHexInt(r, 8, id); formatHexInt(r, 8, id);
return r.toString(); return r.toString();
} }
@@ -484,10 +492,10 @@ public class ChangeUtil {
return Long.parseLong(sortKey, 16); return Long.parseLong(sortKey, 16);
} }
public static void computeSortKey(final Change c) { public static void computeSortKey(Change c) {
long lastUpdated = c.getLastUpdatedOn().getTime(); long lastUpdatedMs = c.getLastUpdatedOn().getTime();
int id = c.getId().get(); int id = c.getId().get();
c.setSortKey(sortKey(lastUpdated, id)); c.setSortKey(sortKey(lastUpdatedMs, id));
} }
public static PatchSet.Id nextPatchSetId(Map<String, Ref> allRefs, public static PatchSet.Id nextPatchSetId(Map<String, Ref> allRefs,

11
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -14,6 +14,9 @@
package com.google.gerrit.server.query.change; package com.google.gerrit.server.query.change;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.DAYS;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
@@ -85,7 +88,9 @@ public abstract class AbstractQueryChangesTest {
protected Account.Id userId; protected Account.Id userId;
protected CurrentUser user; protected CurrentUser user;
protected int clockStepMs = 1; protected int clockStepMs = 1;
private long ts = ChangeUtil.SORT_KEY_EPOCH * 1000; private long clockMs =
MILLISECONDS.convert(ChangeUtil.SORT_KEY_EPOCH_MINS, MINUTES)
+ MILLISECONDS.convert(60, DAYS);
protected abstract Injector createInjector(); protected abstract Injector createInjector();
@@ -416,8 +421,8 @@ public abstract class AbstractQueryChangesTest {
Change change = new Change(new Change.Key(key), id, ownerId, Change change = new Change(new Change.Key(key), id, ownerId,
new Branch.NameKey(project, branch)); new Branch.NameKey(project, branch));
change.setLastUpdatedOn(new Timestamp(ts)); change.setLastUpdatedOn(new Timestamp(clockMs));
ts += clockStepMs; clockMs += clockStepMs;
return changeFactory.create( return changeFactory.create(
projectControlFactory.controlFor(project, projectControlFactory.controlFor(project,
userFactory.create(ownerId)).controlFor(change).getRefControl(), userFactory.create(ownerId)).controlFor(change).getRefControl(),