UBERF-7081 Fix empty collaborative content comparison (#5693)

Signed-off-by: Alexander Onnikov <Alexander.Onnikov@xored.com>
This commit is contained in:
Alexander Onnikov 2024-05-29 18:27:24 +07:00 committed by GitHub
parent ea388c0933
commit 9903d9e7a7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 68 additions and 32 deletions

View File

@ -116,33 +116,53 @@ describe('areEqualMarkups', () => {
expect(areEqualMarkups(markup, markup)).toBeTruthy() expect(areEqualMarkups(markup, markup)).toBeTruthy()
}) })
it('returns true for empty content', async () => { it('returns true for empty content', async () => {
const markup1 = '{"type":"doc","content":[{"type":"paragraph"}]}' expect(
const markup2 = '{"type":"doc","content":[{"type":"paragraph","content":[]}]}' areEqualMarkups('{"type":"doc","content":[]}', '{"type":"doc","content":[{"type":"paragraph"}]}')
expect(areEqualMarkups(markup1, markup2)).toBeTruthy() ).toBeTruthy()
expect(
areEqualMarkups(
'{"type":"doc","content":[{"type":"paragraph"}]}',
'{"type":"doc","content":[{"type":"paragraph","content":[]}]}'
)
).toBeTruthy()
}) })
it('returns true for similar content', async () => { it('returns true for same content but empty marks and attrs', async () => {
const markup1 = '{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello"}]}]}' expect(
const markup2 = areEqualMarkups(
'{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello","content":[],"marks":[],"attrs": {"color": null}}]}]}' '{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello"}]}]}',
expect(areEqualMarkups(markup1, markup2)).toBeTruthy() '{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello","content":[],"marks":[],"attrs": {"color": null}}]}]}'
)
).toBeTruthy()
}) })
it('returns false for the same content with different spaces', async () => { it('returns false for same content but trailing hard breaks', async () => {
const markup1 = '{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello"}]}]}' expect(
const markup2 = areEqualMarkups(
'{"type":"doc","content":[{"type":"hardBreak"},{"type":"paragraph","content":[{"type":"text","text":"hello"}]},{"type":"hardBreak"}]}' '{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello","marks":[]}]}]}',
expect(areEqualMarkups(markup1, markup2)).toBeFalsy() '{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello"},{"type":"hardBreak"}]}]}'
)
).toBeFalsy()
expect(
areEqualMarkups(
'{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello"}]}]}',
'{"type":"doc","content":[{"type":"hardBreak"},{"type":"paragraph","content":[{"type":"text","text":"hello"}]},{"type":"hardBreak"}]}'
)
).toBeFalsy()
}) })
it('returns false for different content', async () => { it('returns false for different content', async () => {
const markup1 = '{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello"}]}]}' expect(
const markup2 = '{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"world"}]}]}' areEqualMarkups(
expect(areEqualMarkups(markup1, markup2)).toBeFalsy() '{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello"}]}]}',
'{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"world"}]}]}'
)
).toBeFalsy()
}) })
it('returns false for different marks', async () => { it('returns false for different marks', async () => {
const markup1 = expect(
'{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello","marks":[{"type":"bold"}]}]}]}' areEqualMarkups(
const markup2 = '{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello","marks":[{"type":"bold"}]}]}]}',
'{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello","marks":[{"type":"italic"}]}]}]}' '{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"hello","marks":[{"type":"italic"}]}]}]}'
expect(areEqualMarkups(markup1, markup2)).toBeFalsy() )
).toBeFalsy()
}) })
}) })

View File

@ -45,7 +45,14 @@ export function areEqualMarkups (markup1: Markup, markup2: Markup): boolean {
return true return true
} }
return equalNodes(markupToJSON(markup1), markupToJSON(markup2)) const node1 = markupToJSON(markup1)
const node2 = markupToJSON(markup2)
if (isEmptyNode(node1) && isEmptyNode(node2)) {
return true
}
return equalNodes(node1, node2)
} }
/** @public */ /** @public */

View File

@ -16,6 +16,7 @@
import { DocumentId, parseDocumentId } from '@hcengineering/collaborator-client' import { DocumentId, parseDocumentId } from '@hcengineering/collaborator-client'
import { isReadonlyDoc } from '@hcengineering/collaboration' import { isReadonlyDoc } from '@hcengineering/collaboration'
import { MeasureContext } from '@hcengineering/core' import { MeasureContext } from '@hcengineering/core'
import { decodeToken } from '@hcengineering/server-token'
import { Extension, onAuthenticatePayload } from '@hocuspocus/server' import { Extension, onAuthenticatePayload } from '@hocuspocus/server'
import { getWorkspaceInfo } from '../account' import { getWorkspaceInfo } from '../account'
@ -34,14 +35,18 @@ export class AuthenticationExtension implements Extension {
async onAuthenticate (data: onAuthenticatePayload): Promise<Context> { async onAuthenticate (data: onAuthenticatePayload): Promise<Context> {
const ctx = this.configuration.ctx const ctx = this.configuration.ctx
const { workspaceUrl, collaborativeDoc } = parseDocumentId(data.documentName as DocumentId) const { workspaceUrl: workspace, collaborativeDoc } = parseDocumentId(data.documentName as DocumentId)
return await ctx.with('authenticate', { workspace }, async () => {
const token = decodeToken(data.token)
ctx.info('authenticate', { workspace, mode: token.extra?.mode ?? '' })
return await ctx.with('authenticate', { workspace: workspaceUrl }, async () => {
// verify workspace can be accessed with the token // verify workspace can be accessed with the token
const workspaceInfo = await getWorkspaceInfo(data.token) const workspaceInfo = await getWorkspaceInfo(data.token)
// verify workspace url in the document matches the token // verify workspace url in the document matches the token
if (workspaceInfo.workspace !== workspaceUrl) { if (workspaceInfo.workspace !== workspace) {
throw new Error('documentName must include workspace') throw new Error('documentName must include workspace')
} }

View File

@ -49,18 +49,21 @@ export class StorageExtension implements Extension {
} }
async onLoadDocument ({ context, documentName }: withContext<onLoadDocumentPayload>): Promise<any> { async onLoadDocument ({ context, documentName }: withContext<onLoadDocumentPayload>): Promise<any> {
this.configuration.ctx.info('load document', { documentName }) const { connectionId } = context
this.configuration.ctx.info('load document', { documentName, connectionId })
return await this.loadDocument(documentName as DocumentId, context) return await this.loadDocument(documentName as DocumentId, context)
} }
async onStoreDocument ({ context, documentName, document }: withContext<onStoreDocumentPayload>): Promise<void> { async onStoreDocument ({ context, documentName, document }: withContext<onStoreDocumentPayload>): Promise<void> {
const { ctx } = this.configuration const { ctx } = this.configuration
const { connectionId } = context
ctx.info('store document', { documentName }) ctx.info('store document', { documentName, connectionId })
const collaborators = this.collaborators.get(documentName) const collaborators = this.collaborators.get(documentName)
if (collaborators === undefined || collaborators.size === 0) { if (collaborators === undefined || collaborators.size === 0) {
ctx.info('no changes for document', { documentName }) ctx.info('no changes for document', { documentName, connectionId })
return return
} }
@ -83,7 +86,7 @@ export class StorageExtension implements Extension {
const collaborators = this.collaborators.get(documentName) const collaborators = this.collaborators.get(documentName)
if (collaborators === undefined || !collaborators.has(connectionId)) { if (collaborators === undefined || !collaborators.has(connectionId)) {
ctx.info('no changes for document', { documentName }) ctx.info('no changes for document', { documentName, connectionId })
return return
} }

View File

@ -178,9 +178,6 @@ export async function start (
return return
} }
const token = decodeToken(authHeader.split(' ')[1])
const context = getContext(token)
const request = req.body as RpcRequest const request = req.body as RpcRequest
const method = methods[request.method] const method = methods[request.method]
if (method === undefined) { if (method === undefined) {
@ -189,6 +186,10 @@ export async function start (
} }
res.status(400).send(response) res.status(400).send(response)
} else { } else {
const token = decodeToken(authHeader.split(' ')[1])
const context = getContext(token)
rpcCtx.info('rpc', { method: request.method, connectionId: context.connectionId, mode: token.extra?.mode ?? '' })
await rpcCtx.with('/rpc', { method: request.method }, async (ctx) => { await rpcCtx.with('/rpc', { method: request.method }, async (ctx) => {
try { try {
const response: RpcResponse = await rpcCtx.with(request.method, {}, async (ctx) => { const response: RpcResponse = await rpcCtx.with(request.method, {}, async (ctx) => {