From bc82afa5b2f483cb4c5ad1ec725736f615eb394d Mon Sep 17 00:00:00 2001 From: "G.Ambatte" Date: Thu, 10 Jul 2025 21:42:51 +1200 Subject: [PATCH 1/3] Split version from hash checks --- server/homebrew.api.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/server/homebrew.api.js b/server/homebrew.api.js index 84f639a4d..009ab408a 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -339,17 +339,22 @@ const api = { // Initialize brew from request and body, destructure query params, and set the initial value for the after-save method const brewFromClient = api.excludePropsFromUpdate(req.body); const brewFromServer = req.brew; + + if(brewFromServer?.version !== brewFromClient?.version){ + console.log(`Version mismatch on brew ${brewFromClient.editId}`); + + res.setHeader('Content-Type', 'application/json'); + return res.status(409).send(JSON.stringify({ message: `The server version is out of sync with the saved brew. Please save your changes elsewhere, refresh, and try again.` })); + } + splitTextStyleAndMetadata(brewFromServer); - + brewFromServer.text = brewFromServer.text.normalize(); brewFromServer.hash = await md5(brewFromServer.text); - if((brewFromServer?.version !== brewFromClient?.version) || (brewFromServer?.hash !== brewFromClient?.hash)) { - if(brewFromClient?.version !== brewFromClient?.version) - console.log(`Version mismatch on brew ${brewFromClient.editId}`); - if(brewFromServer?.hash !== brewFromClient?.hash) { - console.log(`Hash mismatch on brew ${brewFromClient.editId}`); - } + if(brewFromServer?.hash !== brewFromClient?.hash) { + console.log(`Hash mismatch on brew ${brewFromClient.editId}`); + res.setHeader('Content-Type', 'application/json'); return res.status(409).send(JSON.stringify({ message: `The server copy is out of sync with the saved brew. Please save your changes elsewhere, refresh, and try again.` })); } From 7f3a8185580bc460a1de0f36767ae95971caa39b Mon Sep 17 00:00:00 2001 From: "G.Ambatte" Date: Thu, 10 Jul 2025 23:25:57 +1200 Subject: [PATCH 2/3] Add Homebrew API coverage tests --- server/homebrew.api.spec.js | 78 +++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/server/homebrew.api.spec.js b/server/homebrew.api.spec.js index 8bb3a0c0b..e771bd8df 100644 --- a/server/homebrew.api.spec.js +++ b/server/homebrew.api.spec.js @@ -1052,4 +1052,82 @@ brew`); expect(testBrew.tags).toEqual(['tag a']); }); }); + + describe('updateBrew', ()=>{ + it('should return error on version mismatch', async ()=>{ + const brewFromClient = { version: 1 }; + const brewFromServer = { version: 1000 }; + + const req = { + brew : brewFromServer, + body : brewFromClient + }; + + await api.updateBrew(req, res); + + expect(res.status).toHaveBeenCalledWith(409); + expect(res.send).toHaveBeenCalledWith('{\"message\":\"The server version is out of sync with the saved brew. Please save your changes elsewhere, refresh, and try again.\"}'); + }); + + it('should return error on hash mismatch', async ()=>{ + const brewFromClient = { version: 1, hash: '1234' }; + const brewFromServer = { version: 1, text: 'test' }; + + const req = { + brew : brewFromServer, + body : brewFromClient + }; + + await api.updateBrew(req, res); + + expect(req.brew.hash).toBe('098f6bcd4621d373cade4e832627b4f6'); + expect(res.status).toHaveBeenCalledWith(409); + expect(res.send).toHaveBeenCalledWith('{\"message\":\"The server copy is out of sync with the saved brew. Please save your changes elsewhere, refresh, and try again.\"}'); + }); + + it('should return error on applying patches', async ()=>{ + const brewFromClient = { version: 1, hash: '098f6bcd4621d373cade4e832627b4f6', patches: 'not a valid patch string' }; + const brewFromServer = { version: 1, text: 'test', title: 'Test Title', description: 'Test Description' }; + + const req = { + brew : brewFromServer, + body : brewFromClient + }; + + let err; + try { + await api.updateBrew(req, res); + } catch (e) { + err = e; + } + + expect(err).toEqual(Error('Invalid patch string: not a valid patch string')); + }); + + it('should save brew, no ID', async ()=>{ + const brewFromClient = { version: 1, hash: '098f6bcd4621d373cade4e832627b4f6', patches: '' }; + const brewFromServer = { version: 1, text: 'test', title: 'Test Title', description: 'Test Description' }; + + model.save = jest.fn((brew)=>{return brew;}); + + const req = { + brew : brewFromServer, + body : brewFromClient, + query : { saveToGoogle: false, removeFromGoogle: false } + }; + + await api.updateBrew(req, res); + + expect(res.status).toHaveBeenCalledWith(200); + expect(res.send).toHaveBeenCalledWith( + expect.objectContaining({ + _id : '1', + description : 'Test Description', + hash : '098f6bcd4621d373cade4e832627b4f6', + title : 'Test Title', + version : 2 + }) + ); + }); + }); }); From 45689d119e62ab9d11a81a84146e0f236dc6d728 Mon Sep 17 00:00:00 2001 From: Trevor Buckner Date: Thu, 10 Jul 2025 11:18:39 -0400 Subject: [PATCH 3/3] Comment out one failing test Patch failure test is no longer being thrown while we monitor in the background --- server/homebrew.api.spec.js | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/server/homebrew.api.spec.js b/server/homebrew.api.spec.js index e771bd8df..cb953f7e5 100644 --- a/server/homebrew.api.spec.js +++ b/server/homebrew.api.spec.js @@ -1085,24 +1085,25 @@ brew`); expect(res.send).toHaveBeenCalledWith('{\"message\":\"The server copy is out of sync with the saved brew. Please save your changes elsewhere, refresh, and try again.\"}'); }); - it('should return error on applying patches', async ()=>{ - const brewFromClient = { version: 1, hash: '098f6bcd4621d373cade4e832627b4f6', patches: 'not a valid patch string' }; - const brewFromServer = { version: 1, text: 'test', title: 'Test Title', description: 'Test Description' }; + // Commenting this one out for now, since we are no longer throwing this error while we monitor + // it('should return error on applying patches', async ()=>{ + // const brewFromClient = { version: 1, hash: '098f6bcd4621d373cade4e832627b4f6', patches: 'not a valid patch string' }; + // const brewFromServer = { version: 1, text: 'test', title: 'Test Title', description: 'Test Description' }; - const req = { - brew : brewFromServer, - body : brewFromClient - }; + // const req = { + // brew : brewFromServer, + // body : brewFromClient, + // }; - let err; - try { - await api.updateBrew(req, res); - } catch (e) { - err = e; - } + // let err; + // try { + // await api.updateBrew(req, res); + // } catch (e) { + // err = e; + // } - expect(err).toEqual(Error('Invalid patch string: not a valid patch string')); - }); + // expect(err).toEqual(Error('Invalid patch string: not a valid patch string')); + // }); it('should save brew, no ID', async ()=>{ const brewFromClient = { version: 1, hash: '098f6bcd4621d373cade4e832627b4f6', patches: '' };