Skip to content

Change the versions API to always return ALL operations #66

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

Merged
nickbrowne merged 2 commits into from
Nov 13, 2025
Merged

Change the versions API to always return ALL operations #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e48f4cf
Change the versions API to always return ALL operations
nickbrowne Nov 12, 2025
8752b5d
Add some tests to cover the versions endpoint
nickbrowne Nov 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/server/model.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ module.exports = Model = (db, options) ->

buildVersions = (callback, ops, results = []) ->
if ops.length
lastOpV = ops[ops.length - 1].v
Copy link

@mattptc mattptc Nov 12, 2025

Choose a reason for hiding this comment

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

It works as described. How hard would it be to add tests for it?

Copy link
Contributor Author

@nickbrowne nickbrowne Nov 12, 2025

Choose a reason for hiding this comment

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

A bit tedious because they are still nodeunit tests, maybe I'll see if a robot can do it

Copy link

@mattptc mattptc Nov 12, 2025

Choose a reason for hiding this comment

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

This is what Junie came up with

Index: test/model.coffee
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/test/model.coffee b/test/model.coffee
--- a/test/model.coffee	(revision e48f4cfcaccd54498d1cdc241d62ea509d0a101b)
+++ b/test/model.coffee	(date 1762923393660)
@@ -1190,3 +1190,53 @@
             test.deepEqual data.snapshot, "Hello mum"
             test.deepEqual data.v, 3
             test.done()
+
+  'getVersions returns every nth version and includes the latest if not a multiple': (test) ->
+    name = newDocName()
+
+    @model.create name, types.text, (error) =>
+      test.equal error, null
+
+      applyOps @model, name, 0, [
+        [{ p: 0, i: 'Hi' }],        # v1: "Hi"
+        [{ p: 2, i: ' mum' }],      # v2: "Hi mum"
+        [{ p: 1, d: 'i' }],         # v3: "H mum"
+        [{ p: 1, i: 'ello' }],      # v4: "Hello mum"
+      ], (error) =>
+        test.equal error, null
+
+        # n = 2 -> expect versions 2 and 4
+        @model.getVersions name, 2, (error, versions) =>
+          test.equal error, null
+          test.ok Array.isArray versions
+          test.strictEqual versions.length, 2
+
+          test.deepEqual versions[0].v, 2
+          test.strictEqual versions[0].type, 'text'
+          test.deepEqual versions[0].snapshot, 'Hi mum'
+          test.strictEqual typeof versions[0].meta, 'object'
+          test.strictEqual typeof versions[0].meta.ts, 'number'
+
+          test.deepEqual versions[1].v, 4
+          test.strictEqual versions[1].type, 'text'
+          test.deepEqual versions[1].snapshot, 'Hello mum'
+          test.strictEqual typeof versions[1].meta, 'object'
+          test.strictEqual typeof versions[1].meta.ts, 'number'
+
+          # n = 3 -> expect versions 3 and 4 (include latest even if not multiple)
+          @model.getVersions name, 3, (error, versions3) =>
+            test.equal error, null
+            test.ok Array.isArray versions3
+            test.strictEqual versions3.length, 2
+            test.deepEqual versions3.map((d) -> d.v), [3, 4]
+            test.deepEqual versions3.map((d) -> d.snapshot), ['H mum', 'Hello mum']
+            test.done()
+
+  'getVersions on an empty document returns an empty list': (test) ->
+    name = newDocName()
+    @model.create name, types.text, (error) =>
+      test.equal error, null
+      @model.getVersions name, 2, (error, versions) ->
+        test.equal error, null
+        test.deepEqual versions, []
+        test.done()

Copy link
Contributor Author

@nickbrowne nickbrowne Nov 13, 2025

Choose a reason for hiding this comment

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

Took a bit of fiddling, but I've added some tests to at least ensure this continues to work the way it's supposed to.

versionOps = ops.slice(0, opBatchAmount)

try
Expand All @@ -521,7 +522,7 @@ module.exports = Model = (db, options) ->
docTemplate.snapshot = type.apply(docTemplate.snapshot, op.op)
docTemplate.meta = op.meta

if docTemplate.v % n is 0
if docTemplate.v % n is 0 or op.v is lastOpV
results.push({
v: docTemplate.v,
type: docTemplate.type.name,
Expand Down
47 changes: 46 additions & 1 deletion test/rest.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ express = require 'express'
server = require '../src/server'
types = require '../src/types'

{makePassPart, newDocName, fetch} = require './helpers'
{applyOps, makePassPart, newDocName, fetch} = require './helpers'

# Frontend tests
module.exports = testCase
Expand Down Expand Up @@ -84,6 +84,51 @@ module.exports = testCase
test.deepEqual data, 'hi'
test.done()

'GET /doc/:name/versions returns [] for a document with no ops': (test) ->
@model.create @name, 'simple', =>
fetch 'GET', @port, "/doc/#{@name}/versions", null, (res, data, headers) ->
test.strictEqual res.statusCode, 200
test.strictEqual headers['content-type'], 'application/json'
test.deepEqual data, []
test.done()

'GET /doc/:name/versions returns the list of applied ops': (test) ->
@model.create @name, 'text', =>
applyOps @model, @name, 0, [
[{p: 0, i: 'h'}]
[{p: 1, i: 'i'}]
[{p: 2, i: ' '}]
[{p: 3, i: 't'}]
[{p: 4, i: 'h'}]
[{p: 5, i: 'i'}]
[{p: 6, i: 's'}]
[{p: 7, i: ' '}]
[{p: 8, i: 'i'}]
[{p: 9, i: 's'}]
[{p: 10, i: ' '}]
[{p: 11, i: 'd'}]
[{p: 12, i: 'o'}]
[{p: 13, i: 'g'}]
], (error, data) =>
test.strictEqual error, null

fetch 'GET', @port, "/doc/#{@name}/versions?every=10", null, (res, data, headers) ->
test.strictEqual res.statusCode, 200
test.strictEqual headers['content-type'], 'application/json'

version1 = data[0]
version2 = data[1]

test.strictEqual version1.v, 10
test.strictEqual version1.type, 'text'
test.strictEqual version1.snapshot, 'hi this is'

test.strictEqual version2.v, 14
test.strictEqual version2.type, 'text'
test.strictEqual version2.snapshot, 'hi this is dog'

test.done()

'PUT a document creates it': (test) ->
fetch 'PUT', @port, "/doc/#{@name}", {type:'simple'}, (res, data) =>
test.strictEqual res.statusCode, 200
Expand Down