From b88ce2fe118b027c767149291352b01f3e175446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20D=C3=A9nari=C3=A9?= Date: Mon, 4 Dec 2023 17:56:48 +0100 Subject: [PATCH 1/2] fix: Unable to open a document in edition - EXO-67974 Before this fix, sometimes, an entry in OO_EDITOR_CONFIG table is duplicated which lead to a problem when opening a document This fix will ignore the additional entries, and keep only the first one --- .../jpa/storage/impl/RDBMSEditorConfigStorageImpl.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/services/src/main/java/org/exoplatform/onlyoffice/jpa/storage/impl/RDBMSEditorConfigStorageImpl.java b/services/src/main/java/org/exoplatform/onlyoffice/jpa/storage/impl/RDBMSEditorConfigStorageImpl.java index 69ff9d3fc..04d96566e 100644 --- a/services/src/main/java/org/exoplatform/onlyoffice/jpa/storage/impl/RDBMSEditorConfigStorageImpl.java +++ b/services/src/main/java/org/exoplatform/onlyoffice/jpa/storage/impl/RDBMSEditorConfigStorageImpl.java @@ -27,7 +27,8 @@ public Map getConfigsByKey(String key) { List entities = editorConfigDAO.getConfigByKey(key); return entities.stream() .collect(Collectors.toMap(EditorConfigEntity::getEditorUserUserid, - this::buildFromEntity)); + this::buildFromEntity, + (key1, key2) -> key1)); } @Override @@ -36,7 +37,9 @@ public Map getConfigsByDocId(String docId) { List entities = editorConfigDAO.getConfigByDocId(docId); return entities.stream() .collect(Collectors.toConcurrentMap(EditorConfigEntity::getEditorUserUserid, - this::buildFromEntity)); } + this::buildFromEntity, + (key1, key2) -> key1)); + } From 766180f494721d244c6f7f5545a3e85074e81670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20D=C3=A9nari=C3=A9?= Date: Mon, 4 Dec 2023 17:58:10 +0100 Subject: [PATCH 2/2] fix: Re-enforce lock when versionning the node in case of edition with multiple users - EXO-67975 Before this fix, a race condition is possible when a lot of user edit the same document : even if a lock is set to the edited node when creating a new version, the lock allows the current lock owner to pass and then 2 thread (with the same user) can version the current node. This lead to corrupt the node version history and lead to data loss This commit enforce the lock so that it does not let the current lock owner to pass. --- .../OnlyofficeEditorServiceImpl.java | 94 ++++--------------- 1 file changed, 20 insertions(+), 74 deletions(-) diff --git a/services/src/main/java/org/exoplatform/onlyoffice/OnlyofficeEditorServiceImpl.java b/services/src/main/java/org/exoplatform/onlyoffice/OnlyofficeEditorServiceImpl.java index ea7f6e51e..cdaa3e8c6 100644 --- a/services/src/main/java/org/exoplatform/onlyoffice/OnlyofficeEditorServiceImpl.java +++ b/services/src/main/java/org/exoplatform/onlyoffice/OnlyofficeEditorServiceImpl.java @@ -2550,94 +2550,40 @@ protected LockState lock(Node node, Config config) throws OnlyofficeEditorExcept } } - Config.Editor.User user = config.getEditorConfig().getUser(); LockState lock; int attempts = 0; try { do { attempts++; if (node.isLocked()) { + // need wait for unlock if(LOG.isDebugEnabled()) { - LOG.debug("Node (id={},path={}) is already locked. (userId={})", - node.getUUID(), node.getPath(), config.getEditorConfig().getUser().getId()); - } - String lockToken; - try { - lockToken = LockUtil.getLockToken(node); - if(LOG.isDebugEnabled()) { - LOG.debug("Node (id={},path={}) is locked with token={}. (userId={})", - node.getUUID(), node.getPath(),lockToken,config.getEditorConfig().getUser().getId()); - } - } catch (Exception e) { - if (RepositoryException.class.isAssignableFrom(e.getClass())) { - throw RepositoryException.class.cast(e); - } else { - logError(config.getEditorConfig().getUser().getId(), - node.getPath(), - node.getUUID(), - config.getDocument().getKey(), - "Error reading document lock"); - throw new OnlyofficeEditorException("Error reading document lock", e); - } - } - - if (node.getLock().getLockOwner().equals(user.getId()) && lockToken != null) { - if(LOG.isDebugEnabled()) { - LOG.debug("Node (id={},path={}) is locked by currentUser : lockOwner={}, lockToken={}. (userId={})", - node.getUUID(), node.getPath(), node.getLock().getLockOwner(), lockToken, config.getEditorConfig().getUser().getId()); - } - // already this user lock - node.getSession().addLockToken(lockToken); - lock = new LockState(lockToken); - if(LOG.isDebugEnabled()) { - LOG.debug("New lock token ({}) added to node (id={},path={}). (userId={})", - lockToken, - node.getUUID(), - node.getPath(), - config.getEditorConfig().getUser().getId()); - } - } else { - // need wait for unlock - if(LOG.isDebugEnabled()) { - LOG.debug("Wait {} ms before retrying to take lock on node (id={},path={}). (userId={})", - LOCK_WAIT_TIMEOUT, - node.getUUID(), - node.getPath(), - config.getEditorConfig().getUser().getId()); - } - Thread.sleep(LOCK_WAIT_TIMEOUT); - lock = null; + LOG.debug("Wait {} ms before retrying to take lock on node (id={},path={}). (userId={})", + LOCK_WAIT_TIMEOUT, + node.getUUID(), + node.getPath(), + config.getEditorConfig().getUser().getId()); } + Thread.sleep(LOCK_WAIT_TIMEOUT); + lock = null; } else { - if(LOG.isDebugEnabled()) { - LOG.debug("Node (id={},path={}) is not locked, take the lock. (userId={})", - node.getUUID(), node.getPath(), config.getEditorConfig().getUser().getId()); - } - Lock jcrLock = node.lock(true, false); - if(LOG.isDebugEnabled()) { - LOG.debug("Node (id={},path={}) is now locked. (userId={})", - node.getUUID(), node.getPath(), config.getEditorConfig().getUser().getId()); - } - // keep lock token for other sessions of same user try { - LockUtil.keepLock(jcrLock, user.getId(), jcrLock.getLockToken()); - if(LOG.isDebugEnabled()) { - LOG.debug("Node (id={},path={}) is now locked and accept other lock for user. (userId={})", + if (LOG.isDebugEnabled()) { + LOG.debug("Node (id={},path={}) is not locked, take the lock. (userId={})", node.getUUID(), node.getPath(), config.getEditorConfig().getUser().getId()); } - } catch (Exception e) { - if (RepositoryException.class.isAssignableFrom(e.getClass())) { - throw RepositoryException.class.cast(e); - } else { - logError(config.getEditorConfig().getUser().getId(), - node.getPath(), - node.getUUID(), - config.getDocument().getKey(), - "Error saving document lock"); - throw new OnlyofficeEditorException("Error saving document lock", e); + Lock jcrLock = node.lock(true, false); + if (LOG.isDebugEnabled()) { + LOG.debug("Node (id={},path={}) is now locked. (userId={})", + node.getUUID(), node.getPath(), config.getEditorConfig().getUser().getId()); } + lock = new LockState(jcrLock); + } catch (Exception e) { + LOG.warn("Unable to get the lock on node (id={},path={}). Probably because on concurrent access. (userId={})", + node.getUUID(), node.getPath(), config.getEditorConfig().getUser().getId(), e); + lock = null; + } - lock = new LockState(jcrLock); } } while (lock == null && attempts <= LOCK_WAIT_ATTEMTS); } catch (InterruptedException e) {