Skip to content

Commit 87eed84

Browse files
committed
fix(snowflake): Reject time before epoch
This allow to simplify a bit the code, also add a small description of the algorithm. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Carl Schwan <carl@carlschwan.eu>
1 parent 9bffa69 commit 87eed84

6 files changed

Lines changed: 33 additions & 14 deletions

File tree

‎lib/private/Snowflake/FileSequence.php‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,19 @@
1313
use OCP\ITempManager;
1414
use Override;
1515

16+
/**
17+
* Sequence IDs shared between processes through lock files in the temporary directory.
18+
*
19+
* Each lock file is a table of counters. To get a sequence ID, the file chosen by the
20+
* millisecond (ms % NB_FILES) is locked and the slot of that second and millisecond is
21+
* read: ((seconds % SEQUENCE_TTL) * (1000 / NB_FILES) + ms / NB_FILES). A slot holds the
22+
* seconds it was last used for and the last sequence ID handed out. If the seconds match,
23+
* the next sequence ID is that one plus one, otherwise it starts at 0. The result is
24+
* written back and the file unlocked.
25+
*
26+
* A slot is reused SEQUENCE_TTL seconds later, the seconds mismatch then resets its counter.
27+
* No fsync is needed, as the files only coordinate processes running at the same time.
28+
*/
1629
class FileSequence implements ISequence {
1730
/** Number of files to use */
1831
private const NB_FILES = 20;
@@ -72,22 +85,20 @@ public function nextId(int $serverId, int $seconds, int $milliseconds): int {
7285
}
7386

7487
// Each file holds one slot per second of the TTL window and per millisecond mapped to it
75-
$slot = (($seconds % self::SEQUENCE_TTL + self::SEQUENCE_TTL) % self::SEQUENCE_TTL) * intdiv(1000, self::NB_FILES)
76-
+ intdiv($milliseconds, self::NB_FILES);
77-
$slotSecondsKey = $seconds & 0xFFFFFFFF;
88+
$slot = ($seconds % self::SEQUENCE_TTL) * intdiv(1000, self::NB_FILES) + intdiv($milliseconds, self::NB_FILES);
7889
fseek($fp, $slot * self::SLOT_SIZE);
7990
$data = fread($fp, self::SLOT_SIZE);
8091
$sequenceId = 0;
8192
if (is_string($data) && strlen($data) === self::SLOT_SIZE) {
8293
['seconds' => $slotSeconds, 'sequence' => $slotSequenceId] = unpack('Nseconds/Nsequence', $data);
83-
if ($slotSeconds === $slotSecondsKey) {
94+
if ($slotSeconds === $seconds) {
8495
$sequenceId = $slotSequenceId + 1;
8596
}
8697
}
8798

8899
// No fsync needed, the file only coordinates processes running at the same time
89100
fseek($fp, $slot * self::SLOT_SIZE);
90-
fwrite($fp, pack('NN', $slotSecondsKey, $sequenceId));
101+
fwrite($fp, pack('NN', $seconds, $sequenceId));
91102

92103
// Release lock
93104
fclose($fp);

‎lib/private/Snowflake/ISequence.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ public function isAvailable(): bool;
2020

2121
/**
2222
* Returns next sequence ID for current time and server
23+
*
24+
* @param non-negative-int $seconds seconds since the Snowflake epoch
2325
*/
2426
public function nextId(int $serverId, int $seconds, int $milliseconds): int|false;
2527
}

‎lib/private/Snowflake/SnowflakeGenerator.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ public function __construct(
3333
public function nextId(): string {
3434
// Relative time
3535
[$seconds, $milliseconds] = $this->getCurrentTime();
36+
if ($seconds < 0) {
37+
throw new \InvalidArgumentException('Snowflake IDs cannot be generated for a time before ' . date(DATE_ATOM, self::TS_OFFSET));
38+
}
3639

3740
$serverId = $this->getServerId(); // Already 9 bits
3841
$isCli = (int)$this->isCli(); // 1 bit

‎lib/public/Snowflake/ISnowflakeGenerator.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ interface ISnowflakeGenerator {
3939
*
4040
* Each call to this method is guaranteed to return a different ID.
4141
*
42+
* @throws \InvalidArgumentException if the current time is before 2025-10-01, as such a time cannot be represented
4243
* @since 33.0
4344
*/
4445
public function nextId(): string;

‎tests/lib/Snowflake/FileSequenceTest.php‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,11 @@
1212
use OC\Snowflake\FileSequence;
1313
use OCP\ITempManager;
1414

15-
/**
16-
* @package Test
17-
*/
1815
class FileSequenceTest extends ISequenceBase {
1916
private string $path;
2017

2118
public function setUp():void {
19+
parent::setUp();
2220
$tempManager = $this->createMock(ITempManager::class);
2321
$this->path = sys_get_temp_dir() . '/' . uniqid('file_sequence_test_');
2422
mkdir($this->path);
@@ -34,6 +32,7 @@ public function tearDown():void {
3432
rmdir($directory);
3533
}
3634
rmdir($this->path);
35+
parent::tearDown();
3736
}
3837

3938
public function testSequenceIncrementsWithinTheSameMillisecond(): void {
@@ -56,10 +55,4 @@ public function testSequenceKeepsEarlierMillisecondsWithinTheWindow(): void {
5655
$this->assertSame(0, $this->sequence->nextId(42, 1001, 500));
5756
$this->assertSame(1, $this->sequence->nextId(42, 1000, 500));
5857
}
59-
60-
public function testSequenceWithSecondsBeforeTheEpoch(): void {
61-
$this->assertSame(0, $this->sequence->nextId(42, -5, 10));
62-
$this->assertSame(1, $this->sequence->nextId(42, -5, 10));
63-
$this->assertSame(0, $this->sequence->nextId(42, 25, 10));
64-
}
6558
}

‎tests/lib/Snowflake/GeneratorTest.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,15 @@ public function testGenerator(): void {
6363
$this->assertEquals(42, $data->getServerId());
6464
}
6565

66+
public function testGeneratorRejectsTimeBeforeEpoch(): void {
67+
$timeFactory = $this->createMock(ITimeFactory::class);
68+
$timeFactory->method('now')->willReturn(new \DateTimeImmutable('2025-09-30 23:59:59 UTC'));
69+
$generator = new SnowflakeGenerator($timeFactory, $this->config, $this->sequence);
70+
71+
$this->expectException(\InvalidArgumentException::class);
72+
$generator->nextId();
73+
}
74+
6675
#[DataProvider('provideSnowflakeData')]
6776
public function testGeneratorWithFixedTime(string $date, int $expectedSeconds, int $expectedMilliseconds): void {
6877
$dt = new \DateTimeImmutable($date);

0 commit comments

Comments
 (0)