diff --git a/xwiki-platform-core/xwiki-platform-oldcore/pom.xml b/xwiki-platform-core/xwiki-platform-oldcore/pom.xml index e22b357e6f86..e62c934daeb0 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/pom.xml +++ b/xwiki-platform-core/xwiki-platform-oldcore/pom.xml @@ -218,6 +218,12 @@ batik-svgrasterizer + + + com.google.guava + guava + + diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/DefaultDocumentRequiredRightsManager.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/DefaultDocumentRequiredRightsManager.java deleted file mode 100644 index c90f38d7d7fb..000000000000 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/DefaultDocumentRequiredRightsManager.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * See the NOTICE file distributed with this work for additional - * information regarding copyright ownership. - * - * This is free software; you can redistribute it and/or modify it - * under the terms of the GNU Lesser General Public License as - * published by the Free Software Foundation; either version 2.1 of - * the License, or (at your option) any later version. - * - * This software is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this software; if not, write to the Free - * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA - * 02110-1301 USA, or see the FSF site: http://www.fsf.org. - */ -package org.xwiki.internal.document; - -import java.util.Optional; - -import javax.inject.Inject; -import javax.inject.Provider; -import javax.inject.Singleton; - -import org.xwiki.component.annotation.Component; -import org.xwiki.model.reference.DocumentReference; -import org.xwiki.security.authorization.AuthorizationException; -import org.xwiki.security.authorization.requiredrights.DocumentRequiredRights; -import org.xwiki.security.authorization.requiredrights.DocumentRequiredRightsManager; - -import com.xpn.xwiki.XWikiContext; -import com.xpn.xwiki.XWikiException; -import com.xpn.xwiki.doc.XWikiDocument; - -/** - * Default implementation of {@link DocumentRequiredRightsManager}. - * - * @version $Id$ - */ -@Component -@Singleton -public class DefaultDocumentRequiredRightsManager implements DocumentRequiredRightsManager -{ - @Inject - private Provider contextProvider; - - @Inject - private DocumentRequiredRightsReader documentRequiredRightsReader; - - @Override - public Optional getRequiredRights(DocumentReference documentReference) - throws AuthorizationException - { - if (documentReference != null) { - XWikiContext context = this.contextProvider.get(); - - // Load the document. - try { - XWikiDocument document = context.getWiki().getDocument(documentReference.withoutLocale(), context); - if (!document.isNew()) { - return Optional.of(this.documentRequiredRightsReader.readRequiredRights(document)); - } - } catch (XWikiException e) { - throw new AuthorizationException("Failed to load the document", e); - } - } - - return Optional.empty(); - } -} diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/DefaultSimpleDocumentCache.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/DefaultSimpleDocumentCache.java new file mode 100644 index 000000000000..1c0f2e227e94 --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/DefaultSimpleDocumentCache.java @@ -0,0 +1,159 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.internal.document; + +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReadWriteLock; + +import javax.inject.Inject; + +import org.apache.commons.lang3.function.FailableFunction; +import org.xwiki.bridge.event.DocumentCreatedEvent; +import org.xwiki.bridge.event.DocumentDeletedEvent; +import org.xwiki.bridge.event.DocumentUpdatedEvent; +import org.xwiki.cache.Cache; +import org.xwiki.cache.CacheException; +import org.xwiki.cache.CacheManager; +import org.xwiki.cache.config.CacheConfiguration; +import org.xwiki.component.annotation.Component; +import org.xwiki.component.annotation.InstantiationStrategy; +import org.xwiki.component.descriptor.ComponentInstantiationStrategy; +import org.xwiki.component.phase.Disposable; +import org.xwiki.model.reference.DocumentReference; +import org.xwiki.model.reference.EntityReferenceSerializer; +import org.xwiki.observation.AbstractEventListener; +import org.xwiki.observation.EventListener; +import org.xwiki.observation.ObservationManager; +import org.xwiki.observation.event.Event; + +import com.google.common.util.concurrent.Striped; +import com.xpn.xwiki.doc.XWikiDocument; + +/** + * A cache that allows caching a single value per document reference. + * + * @param the type of the data stored in the cache + * @param the type of the exception that can be thrown by the provider + * @version $Id$ + */ +@Component +@InstantiationStrategy(ComponentInstantiationStrategy.PER_LOOKUP) +public class DefaultSimpleDocumentCache implements Disposable, SimpleDocumentCache +{ + @Inject + private ObservationManager observationManager; + + @Inject + private CacheManager cacheManager; + + @Inject + private EntityReferenceSerializer serializer; + + private Cache cache; + + private Listener listener; + + // Use a read-write lock to ensure that during cache invalidation, no values are computed based on outdated data. + // Use a striped lock to ensure that the removal of one value doesn't block the setting of other values. + private final Striped locks = Striped.readWriteLock(16); + + private class Listener extends AbstractEventListener + { + Listener(String name) + { + super(name, new DocumentCreatedEvent(), new DocumentUpdatedEvent(), new DocumentDeletedEvent()); + } + + @Override + public void onEvent(Event event, Object source, Object data) + { + XWikiDocument doc = (XWikiDocument) source; + remove(doc.getDocumentReference()); + } + } + + @Override + public synchronized void initializeCache(CacheConfiguration cacheConfiguration) throws CacheException + { + // If the cache has already been created, dispose the existing one and create a new one. + if (this.cache != null) { + dispose(); + } + + this.cache = this.cacheManager.createNewCache(cacheConfiguration); + this.listener = new Listener(cacheConfiguration.getConfigurationId()); + this.observationManager.addListener(this.listener, EventListener.CACHE_INVALIDATION_DEFAULT_PRIORITY); + } + + @Override + public C get(DocumentReference documentReference, FailableFunction provider) throws E + { + String key = getKey(documentReference); + C result = this.cache.get(key); + + if (result == null) { + // Use a read lock to ensure that we don't compute a new value based on an old document while an + // invalidation is running. We don't care about computing several times in parallel. + Lock lock = this.locks.get(key).readLock(); + lock.lock(); + try { + result = provider.apply(documentReference); + this.cache.set(key, result); + } finally { + lock.unlock(); + } + } + + return result; + } + + /** + * Remove the value associated with the provided document reference. + * + * @param documentReference the reference of the document + */ + public void remove(DocumentReference documentReference) + { + String key = getKey(documentReference); + // Use a write lock to ensure that we don't compute a new value based on an old document. + Lock lock = this.locks.get(key).writeLock(); + lock.lock(); + try { + this.cache.remove(key); + } finally { + lock.unlock(); + } + } + + private String getKey(DocumentReference documentReference) + { + return this.serializer.serialize(documentReference); + } + + @Override + public void dispose() + { + if (this.cache != null) { + this.cache.dispose(); + } + + this.observationManager.removeListener(this.listener.getName()); + } +} diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/DocumentRequiredRightsReader.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/DocumentRequiredRightsReader.java index 266aebbeaf15..86a35cbfa863 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/DocumentRequiredRightsReader.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/DocumentRequiredRightsReader.java @@ -19,6 +19,7 @@ */ package org.xwiki.internal.document; +import java.util.List; import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -60,6 +61,20 @@ public class DocumentRequiredRightsReader */ public static final String PROPERTY_NAME = "level"; + private static final DocumentRequiredRights ENFORCED_EMPTY = new DocumentRequiredRights(true, Set.of()); + + private static final DocumentRequiredRights ENFORCED_SCRIPT = new DocumentRequiredRights(true, + Set.of(new DocumentRequiredRight(Right.SCRIPT, EntityType.DOCUMENT))); + + private static final DocumentRequiredRights ENFORCED_PROGRAMMING = new DocumentRequiredRights(true, + Set.of(new DocumentRequiredRight(Right.PROGRAM, null))); + + private static final DocumentRequiredRights ENFORCED_ADMIN = new DocumentRequiredRights(true, + Set.of(new DocumentRequiredRight(Right.ADMIN, EntityType.WIKI))); + + private static final List STATIC_INSTANCES = List.of(ENFORCED_EMPTY, ENFORCED_SCRIPT, + ENFORCED_PROGRAMMING, ENFORCED_ADMIN); + @Inject private Logger logger; @@ -71,15 +86,33 @@ public class DocumentRequiredRightsReader */ public DocumentRequiredRights readRequiredRights(XWikiDocument document) { - return new DocumentRequiredRights(document.isEnforceRequiredRights(), - document.getXObjects(CLASS_REFERENCE).stream() - .filter(Objects::nonNull) - .map(this::readRequiredRight) - // Don't allow edit right/edit right implies no extra right. - // Filter out invalid values. - .filter(requiredRight -> - !Right.EDIT.equals(requiredRight.right()) && !Right.ILLEGAL.equals(requiredRight.right())) - .collect(Collectors.toUnmodifiableSet())); + boolean enforce = document.isEnforceRequiredRights(); + Set rights = document.getXObjects(CLASS_REFERENCE).stream() + .filter(Objects::nonNull) + .map(this::readRequiredRight) + // Don't allow edit right/edit right implies no extra right. + // Filter out invalid values. + .filter(requiredRight -> + !Right.EDIT.equals(requiredRight.right()) && !Right.ILLEGAL.equals(requiredRight.right())) + .collect(Collectors.toUnmodifiableSet()); + + // Try returning a static instance to avoid creating lots of objects that contain the same values as most + // documents will be in the case of one of the static instances. This is also to reduce the memory usage of + // the cache. + if (!enforce && rights.isEmpty()) { + return DocumentRequiredRights.EMPTY; + } + + // Return the static instance that has the same set of rights. + if (enforce) { + for (DocumentRequiredRights staticInstance : STATIC_INSTANCES) { + if (staticInstance.rights().equals(rights)) { + return staticInstance; + } + } + } + + return new DocumentRequiredRights(enforce, rights); } private DocumentRequiredRight readRequiredRight(BaseObject object) diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/SimpleDocumentCache.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/SimpleDocumentCache.java new file mode 100644 index 000000000000..b86bb97aa77f --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/internal/document/SimpleDocumentCache.java @@ -0,0 +1,59 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.internal.document; + +import org.apache.commons.lang3.function.FailableFunction; +import org.xwiki.cache.CacheException; +import org.xwiki.cache.config.CacheConfiguration; +import org.xwiki.component.annotation.ComponentRole; +import org.xwiki.model.reference.DocumentReference; + +/** + * A simple cache for documents. + * + * @param the type of the cached value + * @param the type of the exception that can be thrown by the provider + * @version $Id$ + */ +// We need to use @ComponentRole here as @Role doesn't support generic component implementations, and there is no +// non-deprecated way to achieve this. +@SuppressWarnings({ "java:S1874", "deprecation" }) +@ComponentRole +public interface SimpleDocumentCache +{ + /** + * Initialize the cache. + * + * @param cacheConfiguration the cache configuration + * @throws CacheException if the cache couldn’t be created + */ + void initializeCache(CacheConfiguration cacheConfiguration) throws CacheException; + + /** + * Get the value associated with the provided document reference, or compute it when missing. The computed valued is + * stored in the cache. + * + * @param documentReference the reference of the document + * @param provider the provider to compute the value if it isn't in the cache. The document used to compute the + * cached value needs to be loaded inside the provider to ensure that no stale data is cached + * @return the value + */ + C get(DocumentReference documentReference, FailableFunction provider) throws E; +} diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/resources/META-INF/components.txt b/xwiki-platform-core/xwiki-platform-oldcore/src/main/resources/META-INF/components.txt index 7513fbc68000..e5ecf16729d5 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/resources/META-INF/components.txt +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/resources/META-INF/components.txt @@ -282,5 +282,5 @@ org.xwiki.evaluation.internal.DefaultObjectEvaluator org.xwiki.evaluation.internal.VelocityObjectPropertyEvaluator org.xwiki.internal.document.DocumentOverrideListener org.xwiki.internal.document.DocumentRequiredRightsReader -org.xwiki.internal.document.DefaultDocumentRequiredRightsManager org.xwiki.internal.document.RequiredRightClassMandatoryDocumentInitializer +org.xwiki.internal.document.DefaultSimpleDocumentCache diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/api/DocumentTest.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/api/DocumentTest.java index 4333f66aa42a..18471a1059db 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/api/DocumentTest.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/api/DocumentTest.java @@ -20,7 +20,6 @@ package com.xpn.xwiki.api; import java.util.List; -import java.util.Optional; import javax.inject.Named; @@ -46,12 +45,10 @@ import org.xwiki.security.authorization.AuthorizationException; import org.xwiki.security.authorization.AuthorizationManager; import org.xwiki.security.authorization.Right; -import org.xwiki.security.authorization.requiredrights.DocumentRequiredRightsManager; import org.xwiki.sheet.SheetBinder; import org.xwiki.test.LogLevel; import org.xwiki.test.annotation.ComponentList; import org.xwiki.test.junit5.LogCaptureExtension; -import org.xwiki.test.junit5.mockito.InjectMockComponents; import org.xwiki.test.junit5.mockito.MockComponent; import org.xwiki.test.mockito.MockitoComponentManager; import org.xwiki.user.CurrentUserReference; @@ -110,29 +107,12 @@ class DocumentTest @MockComponent private ContextualLocalizationManager contextualLocalizationManager; - @MockComponent - private DocumentRequiredRightsManager documentRequiredRightsManager; - - @InjectMockComponents - private DocumentRequiredRightsReader documentRequiredRightsReader; - @RegisterExtension private LogCaptureExtension logCapture = new LogCaptureExtension(LogLevel.INFO); @BeforeEach void setUp() throws AuthorizationException, ComponentLookupException { - when(this.documentRequiredRightsManager.getRequiredRights(any())).then(invocationOnMock -> - { - DocumentReference reference = invocationOnMock.getArgument(0); - XWikiDocument document = - this.oldcore.getSpyXWiki().getDocument(reference.withoutLocale(), this.oldcore.getXWikiContext()); - if (document.isNew()) { - return Optional.empty(); - } - return Optional.of(this.documentRequiredRightsReader.readRequiredRights(document)); - }); - this.oldcore.getSpyXWiki().initializeMandatoryDocuments(this.oldcore.getXWikiContext()); DefaultParameterizedType currentUserReferenceResolverType = diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/test/MockitoOldcore.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/test/MockitoOldcore.java index 6fc16e616e83..903f5cfcfae5 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/test/MockitoOldcore.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/test/MockitoOldcore.java @@ -31,6 +31,7 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import javax.inject.Provider; @@ -59,6 +60,7 @@ import org.xwiki.context.ExecutionContextManager; import org.xwiki.environment.Environment; import org.xwiki.environment.internal.ServletEnvironment; +import org.xwiki.internal.document.DocumentRequiredRightsReader; import org.xwiki.model.document.DocumentAuthors; import org.xwiki.model.internal.reference.EntityReferenceFactory; import org.xwiki.model.reference.DocumentReference; @@ -78,6 +80,7 @@ import org.xwiki.security.authorization.AuthorizationManager; import org.xwiki.security.authorization.ContextualAuthorizationManager; import org.xwiki.security.authorization.DocumentAuthorizationManager; +import org.xwiki.security.authorization.requiredrights.DocumentRequiredRightsManager; import org.xwiki.test.TestEnvironment; import org.xwiki.test.annotation.AllComponents; import org.xwiki.test.internal.MockConfigurationSource; @@ -1084,6 +1087,29 @@ public Boolean answer(InvocationOnMock invocation) throws Throwable }); } + // Add a DocumentRequiredRightsManager if we have a DocumentRequiredRightsReader as the former isn't easily + // available in a mocked setup while the latter can be loaded without problems. + if (!this.componentManager.hasComponent(DocumentRequiredRightsManager.class) + && this.componentManager.hasComponent(DocumentRequiredRightsReader.class)) { + DocumentRequiredRightsManager requiredRightsManager = + this.componentManager.registerMockComponent(DocumentRequiredRightsManager.class); + DocumentRequiredRightsReader documentRequiredRightsReader = + this.componentManager.getInstance(DocumentRequiredRightsReader.class); + + when(requiredRightsManager.getRequiredRights(any())).then(invocationOnMock -> + { + DocumentReference reference = invocationOnMock.getArgument(0); + if (reference != null) { + XWikiDocument document = getSpyXWiki().getDocument(reference.withoutLocale(), getXWikiContext()); + if (!document.isNew()) { + return Optional.of(documentRequiredRightsReader.readRequiredRights(document)); + } + } + + return Optional.empty(); + }); + } + // Query Manager // If there's already a Query Manager registered, use it instead. // This allows, for example, using @ComponentList to use the real Query Manager, in integration tests. diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/org/xwiki/internal/document/DefaultDocumentRequiredRightsManagerTest.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/org/xwiki/internal/document/DefaultDocumentRequiredRightsManagerTest.java deleted file mode 100644 index b13f3659d003..000000000000 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/org/xwiki/internal/document/DefaultDocumentRequiredRightsManagerTest.java +++ /dev/null @@ -1,114 +0,0 @@ -/* - * See the NOTICE file distributed with this work for additional - * information regarding copyright ownership. - * - * This is free software; you can redistribute it and/or modify it - * under the terms of the GNU Lesser General Public License as - * published by the Free Software Foundation; either version 2.1 of - * the License, or (at your option) any later version. - * - * This software is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this software; if not, write to the Free - * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA - * 02110-1301 USA, or see the FSF site: http://www.fsf.org. - */ -package org.xwiki.internal.document; - -import java.util.Locale; - -import org.junit.jupiter.api.Test; -import org.xwiki.model.reference.DocumentReference; -import org.xwiki.security.authorization.AuthorizationException; -import org.xwiki.security.authorization.requiredrights.DocumentRequiredRights; -import org.xwiki.test.junit5.mockito.InjectMockComponents; -import org.xwiki.test.junit5.mockito.MockComponent; - -import com.xpn.xwiki.XWikiException; -import com.xpn.xwiki.doc.XWikiDocument; -import com.xpn.xwiki.test.MockitoOldcore; -import com.xpn.xwiki.test.junit5.mockito.InjectMockitoOldcore; -import com.xpn.xwiki.test.junit5.mockito.OldcoreTest; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.when; - -/** - * Component test for {@link DefaultDocumentRequiredRightsManager}. - * - * @version $Id$ - */ -@OldcoreTest -class DefaultDocumentRequiredRightsManagerTest -{ - private static final DocumentReference DOCUMENT_REFERENCE = new DocumentReference("wiki", "space", "page"); - - @InjectMockitoOldcore - private MockitoOldcore mockitoOldcore; - - @InjectMockComponents - private DefaultDocumentRequiredRightsManager documentRequiredRightsManager; - - @MockComponent - private DocumentRequiredRightsReader documentRequiredRightsReader; - - @Test - void existingDocument() throws XWikiException, AuthorizationException - { - XWikiDocument document = - this.mockitoOldcore.getSpyXWiki().getDocument(DOCUMENT_REFERENCE, this.mockitoOldcore.getXWikiContext()); - this.mockitoOldcore.getSpyXWiki().saveDocument(document, this.mockitoOldcore.getXWikiContext()); - - DocumentRequiredRights documentRequiredRights = mock(); - when(this.documentRequiredRightsReader.readRequiredRights(document)).thenReturn(documentRequiredRights); - - assertEquals(documentRequiredRights, - this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE).orElseThrow()); - } - - @Test - void documentWithLocale() throws XWikiException, AuthorizationException - { - XWikiDocument document = - this.mockitoOldcore.getSpyXWiki().getDocument(DOCUMENT_REFERENCE, this.mockitoOldcore.getXWikiContext()); - this.mockitoOldcore.getSpyXWiki().saveDocument(document, this.mockitoOldcore.getXWikiContext()); - - DocumentRequiredRights documentRequiredRights = mock(); - when(this.documentRequiredRightsReader.readRequiredRights(document)).thenReturn(documentRequiredRights); - - DocumentReference documentReferenceWithLocale = new DocumentReference(DOCUMENT_REFERENCE, Locale.GERMAN); - - assertSame(documentRequiredRights, - this.documentRequiredRightsManager.getRequiredRights(documentReferenceWithLocale).orElseThrow()); - } - - @Test - void missingDocument() throws AuthorizationException - { - assertTrue(this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE).isEmpty()); - verifyNoInteractions(this.documentRequiredRightsReader); - } - - @Test - void failedLoad() throws XWikiException - { - XWikiException expected = mock(); - when(this.mockitoOldcore.getSpyXWiki().getDocument(DOCUMENT_REFERENCE, this.mockitoOldcore.getXWikiContext())) - .thenThrow(expected); - - AuthorizationException actual = assertThrows(AuthorizationException.class, - () -> this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE)); - - assertEquals("Failed to load the document", actual.getMessage()); - assertEquals(expected, actual.getCause()); - } -} diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/org/xwiki/internal/document/DefaultSimpleDocumentCacheTest.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/org/xwiki/internal/document/DefaultSimpleDocumentCacheTest.java new file mode 100644 index 000000000000..9470a18828bd --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/org/xwiki/internal/document/DefaultSimpleDocumentCacheTest.java @@ -0,0 +1,393 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.internal.document; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.xwiki.bridge.event.DocumentUpdatedEvent; +import org.xwiki.cache.Cache; +import org.xwiki.cache.CacheException; +import org.xwiki.cache.CacheManager; +import org.xwiki.cache.config.LRUCacheConfiguration; +import org.xwiki.model.reference.DocumentReference; +import org.xwiki.model.reference.EntityReferenceSerializer; +import org.xwiki.observation.EventListener; +import org.xwiki.observation.ObservationManager; +import org.xwiki.test.junit5.mockito.ComponentTest; +import org.xwiki.test.junit5.mockito.InjectMockComponents; +import org.xwiki.test.junit5.mockito.MockComponent; + +import com.xpn.xwiki.XWikiContext; +import com.xpn.xwiki.XWikiException; +import com.xpn.xwiki.doc.XWikiDocument; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link DefaultSimpleDocumentCache}. + * + * @version $Id$ + */ +// Tests can depend on many classes. +@SuppressWarnings("checkstyle:ClassFanOutComplexity") +@ComponentTest +class DefaultSimpleDocumentCacheTest +{ + private static final LRUCacheConfiguration CACHE_CONFIGURATION = new LRUCacheConfiguration("test", 100); + + private static final DocumentReference DOCUMENT_REFERENCE = new DocumentReference("wiki", "space", "page"); + + private static final String DOCUMENT_REFERENCE_STRING = "wiki:space.page"; + + @InjectMockComponents + private DefaultSimpleDocumentCache cache; + + @Mock + private Cache internalCache; + + @MockComponent + private ObservationManager observationManager; + + @MockComponent + private CacheManager cacheManager; + + @MockComponent + private EntityReferenceSerializer entityReferenceSerializer; + + @BeforeEach + void setUp() throws Exception + { + doReturn(this.internalCache).when(this.cacheManager).createNewCache(any()); + when(this.entityReferenceSerializer.serialize(DOCUMENT_REFERENCE)).thenReturn(DOCUMENT_REFERENCE_STRING); + } + + @Test + void initializeCache() throws CacheException + { + this.cache.initializeCache(CACHE_CONFIGURATION); + verify(this.cacheManager).createNewCache(CACHE_CONFIGURATION); + verify(this.observationManager).addListener(any(), eq(EventListener.CACHE_INVALIDATION_DEFAULT_PRIORITY)); + } + + @Test + void getMissing() throws Exception + { + this.cache.initializeCache(CACHE_CONFIGURATION); + String value = "missing value"; + assertEquals(value, this.cache.get(DOCUMENT_REFERENCE, documentReference -> value)); + verify(this.internalCache).get(DOCUMENT_REFERENCE_STRING); + verify(this.internalCache).set(DOCUMENT_REFERENCE_STRING, value); + } + + @Test + void getExisting() throws Exception + { + this.cache.initializeCache(CACHE_CONFIGURATION); + String value = "stored value"; + when(this.internalCache.get(DOCUMENT_REFERENCE_STRING)).thenReturn(value); + assertEquals(value, this.cache.get(DOCUMENT_REFERENCE, documentReference -> "should not be called")); + verify(this.internalCache).get(DOCUMENT_REFERENCE_STRING); + verify(this.internalCache, never()).set(any(), any()); + } + + @Test + void getThrows() throws CacheException + { + this.cache.initializeCache(CACHE_CONFIGURATION); + XWikiException exception = new XWikiException(); + XWikiException thrownException = assertThrows(XWikiException.class, + () -> this.cache.get(DOCUMENT_REFERENCE, documentReference -> { + throw exception; + })); + assertSame(exception, thrownException); + verify(this.internalCache).get(DOCUMENT_REFERENCE_STRING); + verify(this.internalCache, never()).set(any(), any()); + } + + @Test + void removeOnEvent() throws CacheException + { + this.cache.initializeCache(CACHE_CONFIGURATION); + // Capture the event listener that got added. + ArgumentCaptor listenerCaptor = ArgumentCaptor.captor(); + verify(this.observationManager).addListener(listenerCaptor.capture(), + eq(EventListener.CACHE_INVALIDATION_DEFAULT_PRIORITY)); + XWikiDocument updatedDocument = new XWikiDocument(DOCUMENT_REFERENCE); + XWikiContext mockContext = mock(); + listenerCaptor.getValue().onEvent(new DocumentUpdatedEvent(DOCUMENT_REFERENCE), updatedDocument, mockContext); + verify(this.internalCache).remove(DOCUMENT_REFERENCE_STRING); + } + + @Test + void updateWaitsOnRemoval() throws Exception + { + this.cache.initializeCache(CACHE_CONFIGURATION); + String value = "value of updateWaitsOnRemoval"; + + CompletableFuture blockRemovalFuture = new CompletableFuture<>(); + CompletableFuture arrivedInRemovalFuture = new CompletableFuture<>(); + CompletableFuture arrivedInGetFuture = new CompletableFuture<>(); + CompletableFuture arrivedInSetFuture = new CompletableFuture<>(); + CompletableFuture arrivedInProviderFuture = new CompletableFuture<>(); + CompletableFuture providerFuture = new CompletableFuture<>(); + + // Initialize an executor with two threads to allow parallel blocking operations. + ExecutorService executor = Executors.newFixedThreadPool(2); + + try { + doAnswer(invocationOnMock -> { + arrivedInRemovalFuture.complete(null); + blockRemovalFuture.get(); + return null; + }).when(this.internalCache).remove(DOCUMENT_REFERENCE_STRING); + + when(this.internalCache.get(DOCUMENT_REFERENCE_STRING)).then(invocationOnMock -> { + arrivedInGetFuture.complete(null); + return null; + }); + + doAnswer(invocationOnMock -> { + arrivedInSetFuture.complete(null); + return null; + }).when(this.internalCache).set(DOCUMENT_REFERENCE_STRING, value); + + CompletableFuture removeFuture = + CompletableFuture.runAsync(() -> this.cache.remove(DOCUMENT_REFERENCE), executor); + + // Wait for the thread to arrive in the actual remove method of the cache. Only wait for 10 seconds to not + // block the test forever in case the test should fail. + arrivedInRemovalFuture.get(10, TimeUnit.SECONDS); + + CompletableFuture updateFuture = CompletableFuture.runAsync(() -> { + try { + this.cache.get(DOCUMENT_REFERENCE, documentReference -> { + arrivedInProviderFuture.complete(null); + return providerFuture.get(); + }); + } catch (Exception e) { + throw new RuntimeException(e); + } + }, executor); + + arrivedInGetFuture.get(10, TimeUnit.SECONDS); + + // Ensure that we don't arrive in the provider or the set method even when waiting for a second. + assertThrows(TimeoutException.class, + () -> CompletableFuture.anyOf(arrivedInProviderFuture, arrivedInSetFuture).get(1, TimeUnit.SECONDS)); + + // Unblock the removal operation. + blockRemovalFuture.complete(null); + + // Wait for the removal to complete as it should be unblocked now. + removeFuture.get(10, TimeUnit.SECONDS); + + // Ensure that we're getting into the provider now. + arrivedInProviderFuture.get(10, TimeUnit.SECONDS); + + // Unblock the provider operation. + providerFuture.complete(value); + + // Ensure that we're getting into the set method now. + arrivedInSetFuture.get(10, TimeUnit.SECONDS); + + // Wait for the update operation to complete. + updateFuture.get(10, TimeUnit.SECONDS); + + verify(this.internalCache).set(DOCUMENT_REFERENCE_STRING, value); + } finally { + executor.shutdownNow(); + } + } + + @Test + void removalWaitsOnUpdate() throws Exception + { + this.cache.initializeCache(CACHE_CONFIGURATION); + String value = "value of removalWaitsOnUpdate"; + + CompletableFuture arrivedInProviderFuture = new CompletableFuture<>(); + CompletableFuture providerFuture = new CompletableFuture<>(); + CompletableFuture arrivedInRemovalFuture = new CompletableFuture<>(); + + // Initialize an executor with two threads to allow parallel blocking operations. + ExecutorService executor = Executors.newFixedThreadPool(2); + + try { + doAnswer(invocationOnMock -> { + arrivedInRemovalFuture.complete(null); + return null; + }).when(this.internalCache).remove(DOCUMENT_REFERENCE_STRING); + + CompletableFuture updateFuture = CompletableFuture.runAsync(() -> { + try { + this.cache.get(DOCUMENT_REFERENCE, documentReference -> { + arrivedInProviderFuture.complete(null); + return providerFuture.get(20, TimeUnit.SECONDS); + }); + } catch (Exception e) { + throw new RuntimeException(e); + } + }, executor); + + // Ensure that the cache is blocked in the provider. + arrivedInProviderFuture.get(10, TimeUnit.SECONDS); + + CompletableFuture removeFuture = + CompletableFuture.runAsync(() -> this.cache.remove(DOCUMENT_REFERENCE), executor); + + // Ensure that we don't arrive in the removal method even when waiting for a second. + assertThrows(TimeoutException.class, () -> arrivedInRemovalFuture.get(1, TimeUnit.SECONDS)); + + // Unblock the set operation. + providerFuture.complete(value); + + // Wait for the set operation to complete as it should be unblocked now. + updateFuture.get(10, TimeUnit.SECONDS); + + // Ensure that we're getting into the removal method now. + arrivedInRemovalFuture.get(10, TimeUnit.SECONDS); + + // Wait for the removal operation to complete. + removeFuture.get(10, TimeUnit.SECONDS); + + verify(this.internalCache).remove(DOCUMENT_REFERENCE_STRING); + verify(this.internalCache).set(DOCUMENT_REFERENCE_STRING, value); + } finally { + executor.shutdownNow(); + } + } + + /** + * Test that two requests to get() with the same key don't block each other, that both can access the cache at the + * same time and also provide values at the same time. + */ + @Test + void parallelGetRequestsDontBlock() throws Exception + { + this.cache.initializeCache(CACHE_CONFIGURATION); + + List values = List.of("0", "1"); + + List> arrivedInGetFuture = + List.of(new CompletableFuture<>(), new CompletableFuture<>()); + List> blockGetFuture = + List.of(new CompletableFuture<>(), new CompletableFuture<>()); + List> arrivedInProviderFuture = + List.of(new CompletableFuture<>(), new CompletableFuture<>()); + List> blockProviderFuture = + List.of(new CompletableFuture<>(), new CompletableFuture<>()); + List> arrivedInSetFuture = + List.of(new CompletableFuture<>(), new CompletableFuture<>()); + List> blockSetFuture = List.of(new CompletableFuture<>(), new CompletableFuture<>()); + + when(this.internalCache.get(DOCUMENT_REFERENCE_STRING)) + .then(invocationOnMock -> { + arrivedInGetFuture.get(0).complete(null); + return blockGetFuture.get(0).get(20, TimeUnit.SECONDS); + }) + .then(invocationOnMock -> { + arrivedInGetFuture.get(1).complete(null); + return blockGetFuture.get(1).get(20, TimeUnit.SECONDS); + }); + + doAnswer(invocationOnMock -> { + int value = Integer.parseInt(invocationOnMock.getArgument(1)); + arrivedInSetFuture.get(value).complete(null); + blockSetFuture.get(value).get(20, TimeUnit.SECONDS); + return null; + }).when(this.internalCache).set(eq(DOCUMENT_REFERENCE_STRING), any()); + + // Initialize an executor with two threads to allow parallel blocking operations. + ExecutorService executor = Executors.newFixedThreadPool(2); + + try { + List> getFuture = new ArrayList<>(2); + + for (int i = 0; i < 2; ++i) { + int finalI = i; + getFuture.add(CompletableFuture.runAsync(() -> { + try { + this.cache.get(DOCUMENT_REFERENCE, documentReference -> { + arrivedInProviderFuture.get(finalI).complete(null); + return blockProviderFuture.get(finalI).get(20, TimeUnit.SECONDS); + }); + } catch (Exception e) { + throw new RuntimeException(e); + } + }, executor)); + + // Wait for the thread to arrive in the actual get method of the cache. Only wait for 10 seconds to not + // block the test forever in case the test should fail. + arrivedInGetFuture.get(i).get(10, TimeUnit.SECONDS); + } + + for (int i = 0; i < 2; ++i) { + // Unblock one thread after the other and let it call the provider. + blockGetFuture.get(i).complete(null); + + // Wait for the thread to arrive in the provider. + arrivedInProviderFuture.get(i).get(10, TimeUnit.SECONDS); + } + + for (int i = 1; i >= 0; --i) { + // Unblock the provider for both threads and let them call the set method. Do it in the reverse order + // to ensure that it doesn't depend on the order. + blockProviderFuture.get(i).complete(values.get(i)); + + // Wait for the thread to arrive in the set method. + arrivedInSetFuture.get(i).get(10, TimeUnit.SECONDS); + } + + for (int i = 0; i < 2; ++i) { + // Unblock the set method for both threads. + blockSetFuture.get(i).complete(null); + + // Wait for the thread to complete. + getFuture.get(i).get(10, TimeUnit.SECONDS); + } + + for (String value : values) { + verify(this.internalCache).set(DOCUMENT_REFERENCE_STRING, value); + } + } finally { + executor.shutdownNow(); + } + } +} diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authorization/xwiki-platform-security-authorization-bridge/src/main/java/org/xwiki/security/authorization/internal/DefaultDocumentRequiredRightsManager.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authorization/xwiki-platform-security-authorization-bridge/src/main/java/org/xwiki/security/authorization/internal/DefaultDocumentRequiredRightsManager.java new file mode 100644 index 000000000000..4b3eff8fe3d8 --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authorization/xwiki-platform-security-authorization-bridge/src/main/java/org/xwiki/security/authorization/internal/DefaultDocumentRequiredRightsManager.java @@ -0,0 +1,142 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.security.authorization.internal; + +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; + +import javax.inject.Inject; +import javax.inject.Provider; +import javax.inject.Singleton; + +import org.apache.commons.lang3.exception.ExceptionUtils; +import org.slf4j.Logger; +import org.xwiki.cache.config.LRUCacheConfiguration; +import org.xwiki.component.annotation.Component; +import org.xwiki.internal.document.DocumentRequiredRightsReader; +import org.xwiki.internal.document.SimpleDocumentCache; +import org.xwiki.model.reference.DocumentReference; +import org.xwiki.security.authorization.AuthorizationException; +import org.xwiki.security.authorization.requiredrights.DocumentRequiredRights; +import org.xwiki.security.authorization.requiredrights.DocumentRequiredRightsManager; + +import com.xpn.xwiki.XWikiContext; +import com.xpn.xwiki.XWikiException; +import com.xpn.xwiki.doc.XWikiDocument; + +/** + * Default implementation of {@link DocumentRequiredRightsManager}. + * + * @version $Id$ + */ +@Component +@Singleton +public class DefaultDocumentRequiredRightsManager implements DocumentRequiredRightsManager +{ + /** + * Default capacity of the required rights cache. The cache size is rather large to avoid issues on larger wikis. + * Further, even non-existing pages can be cached, and the cached values should be mostly very small, so the + * primary memory usage is from the keys (document references). + */ + private static final int DEFAULT_CAPACITY = 10000; + + @Inject + private Provider contextProvider; + + @Inject + private DocumentRequiredRightsReader documentRequiredRightsReader; + + @Inject + private SimpleDocumentCache, AuthorizationException> cache; + + @Inject + private Logger logger; + + private final AtomicBoolean initializing = new AtomicBoolean(); + + private final AtomicBoolean initialized = new AtomicBoolean(); + + @Override + public Optional getRequiredRights(DocumentReference documentReference) + throws AuthorizationException + { + if (documentReference != null) { + // The cache is initialized on demand as initializing the cache during the initialization of the + // components creates component loading cycles. The main problem is that the authorization manager is + // injected before event listeners have been initialized and is also injected in event listeners, and + // initializing the cache triggers the event listener initialization. In the event listener, another + // instance of the required rights manager is then created. As these cycles are hard to break and might + // also occur in extensions, this component is designed to be as safe as possible in this regard. + if (this.initialized.getAcquire() || tryInitializeCache()) { + return this.cache.get(documentReference.withoutLocale(), this::loadRequiredRights); + } + + // Continue without cache if the cache failed to initialize or is currently being initialized by another + // thread or a right check is triggered by the cache initialization itself. + return loadRequiredRights(documentReference.withoutLocale()); + } + + return Optional.empty(); + } + + private Optional loadRequiredRights(DocumentReference documentReference) + throws AuthorizationException + { + XWikiContext context = this.contextProvider.get(); + + // Load the document. + try { + XWikiDocument document = context.getWiki().getDocument(documentReference, context); + if (!document.isNew()) { + return Optional.of(this.documentRequiredRightsReader.readRequiredRights(document)); + } + } catch (XWikiException e) { + throw new AuthorizationException("Failed to load the document", e); + } + + return Optional.empty(); + } + + private boolean tryInitializeCache() + { + if (!this.initializing.compareAndExchange(false, true)) { + try { + if (!this.initialized.get()) { + LRUCacheConfiguration cacheConfiguration = + new LRUCacheConfiguration("platform.security.authorization.requiredrights.cache", + DEFAULT_CAPACITY); + this.cache.initializeCache(cacheConfiguration); + this.initialized.set(true); + } + + // Return true if either the cache has been initialized or it had already been initialized before. + return true; + } catch (Exception e) { + this.logger.warn("Failed to initialize the required rights cache: [{}]", + ExceptionUtils.getRootCauseMessage(e)); + this.logger.debug("Full stack trace of required rights cache initialization failure:", e); + } finally { + this.initializing.set(false); + } + } + + return false; + } +} diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authorization/xwiki-platform-security-authorization-bridge/src/main/resources/META-INF/components.txt b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authorization/xwiki-platform-security-authorization-bridge/src/main/resources/META-INF/components.txt index 21e8972c8dd9..2ccd6843cbd4 100644 --- a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authorization/xwiki-platform-security-authorization-bridge/src/main/resources/META-INF/components.txt +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authorization/xwiki-platform-security-authorization-bridge/src/main/resources/META-INF/components.txt @@ -1,6 +1,7 @@ 500:org.xwiki.security.authorization.internal.BridgeAuthorizationManager org.xwiki.security.authorization.internal.DefaultContextualAuthorizationManager org.xwiki.security.authorization.internal.DefaultDocumentAuthorizationManager +org.xwiki.security.authorization.internal.DefaultDocumentRequiredRightsManager org.xwiki.security.authorization.internal.DefaultSecurityCacheRulesInvalidator org.xwiki.security.authorization.internal.DefaultSecurityCacheRulesInvalidatorListener org.xwiki.security.authorization.internal.DefaultSecurityEntryReader diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authorization/xwiki-platform-security-authorization-bridge/src/test/java/org/xwiki/security/authorization/internal/DefaultDocumentRequiredRightsManagerTest.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authorization/xwiki-platform-security-authorization-bridge/src/test/java/org/xwiki/security/authorization/internal/DefaultDocumentRequiredRightsManagerTest.java new file mode 100644 index 000000000000..d01963839e76 --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authorization/xwiki-platform-security-authorization-bridge/src/test/java/org/xwiki/security/authorization/internal/DefaultDocumentRequiredRightsManagerTest.java @@ -0,0 +1,250 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.security.authorization.internal; + +import java.util.Locale; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import org.apache.commons.lang3.function.FailableFunction; +import org.apache.commons.lang3.mutable.Mutable; +import org.apache.commons.lang3.mutable.MutableObject; +import org.junit.jupiter.api.Test; +import org.xwiki.internal.document.DocumentRequiredRightsReader; +import org.xwiki.internal.document.SimpleDocumentCache; +import org.xwiki.model.reference.DocumentReference; +import org.xwiki.security.authorization.AuthorizationException; +import org.xwiki.security.authorization.requiredrights.DocumentRequiredRights; +import org.xwiki.test.annotation.BeforeComponent; +import org.xwiki.test.junit5.mockito.InjectMockComponents; +import org.xwiki.test.junit5.mockito.MockComponent; +import org.xwiki.test.mockito.MockitoComponentManager; + +import com.xpn.xwiki.XWikiException; +import com.xpn.xwiki.doc.XWikiDocument; +import com.xpn.xwiki.test.MockitoOldcore; +import com.xpn.xwiki.test.junit5.mockito.InjectMockitoOldcore; +import com.xpn.xwiki.test.junit5.mockito.OldcoreTest; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +/** + * Component test for {@link DefaultDocumentRequiredRightsManager}. + * + * @version $Id$ + */ +// Tests can use many different classes. +@SuppressWarnings("checkstyle:ClassFanOutComplexity") +@OldcoreTest +class DefaultDocumentRequiredRightsManagerTest +{ + private static final DocumentReference DOCUMENT_REFERENCE = new DocumentReference("wiki", "space", "page"); + + @InjectMockitoOldcore + private MockitoOldcore mockitoOldcore; + + @InjectMockComponents + private DefaultDocumentRequiredRightsManager documentRequiredRightsManager; + + @MockComponent + private DocumentRequiredRightsReader documentRequiredRightsReader; + + private SimpleDocumentCache, AuthorizationException> simpleDocumentCache; + + @BeforeComponent + void beforeComponent(MockitoComponentManager componentManager) throws Exception + { + // We intentionally register the component without any type as it registered with @ComponentRole to not consider + // generics. + this.simpleDocumentCache = componentManager.registerMockComponent(SimpleDocumentCache.class); + when(this.simpleDocumentCache.get(any(), any())).then(invocationOnMock -> { + FailableFunction function = + invocationOnMock.getArgument(1); + return function.apply(invocationOnMock.getArgument(0)); + } + ); + } + + @Test + void nullDocument() throws AuthorizationException + { + assertTrue(this.documentRequiredRightsManager.getRequiredRights(null).isEmpty()); + verifyNoInteractions(this.documentRequiredRightsReader); + verifyNoInteractions(this.simpleDocumentCache); + } + + @Test + void existingDocument() throws Exception + { + DocumentRequiredRights documentRequiredRights = initializeMockDocument(); + + assertEquals(documentRequiredRights, + this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE).orElseThrow()); + + verify(this.simpleDocumentCache).initializeCache(any()); + verify(this.simpleDocumentCache).get(eq(DOCUMENT_REFERENCE), any()); + } + + @Test + void documentWithLocale() throws Exception + { + DocumentRequiredRights documentRequiredRights = initializeMockDocument(); + + DocumentReference documentReferenceWithLocale = new DocumentReference(DOCUMENT_REFERENCE, Locale.GERMAN); + + assertSame(documentRequiredRights, + this.documentRequiredRightsManager.getRequiredRights(documentReferenceWithLocale).orElseThrow()); + + verify(this.simpleDocumentCache).initializeCache(any()); + verify(this.simpleDocumentCache).get(eq(DOCUMENT_REFERENCE), any()); + } + + @Test + void missingDocument() throws Exception + { + assertTrue(this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE).isEmpty()); + verifyNoInteractions(this.documentRequiredRightsReader); + + verify(this.simpleDocumentCache).initializeCache(any()); + verify(this.simpleDocumentCache).get(eq(DOCUMENT_REFERENCE), any()); + } + + @Test + void failedLoad() throws Exception + { + XWikiException expected = mock(); + when(this.mockitoOldcore.getSpyXWiki().getDocument(DOCUMENT_REFERENCE, this.mockitoOldcore.getXWikiContext())) + .thenThrow(expected); + + AuthorizationException actual = assertThrows(AuthorizationException.class, + () -> this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE)); + + assertEquals("Failed to load the document", actual.getMessage()); + assertEquals(expected, actual.getCause()); + + verify(this.simpleDocumentCache).initializeCache(any()); + verify(this.simpleDocumentCache).get(eq(DOCUMENT_REFERENCE), any()); + } + + /** + * Verify that two calls that if the initialization is ongoing, the required rights manager still answers as + * expected. + */ + @Test + void parallelLoad() throws Exception + { + CompletableFuture arrivedInInitializeCacheFuture = new CompletableFuture<>(); + CompletableFuture blockInitializeCacheFuture = new CompletableFuture<>(); + + doAnswer(invocationOnMock -> { + arrivedInInitializeCacheFuture.complete(null); + blockInitializeCacheFuture.get(20, TimeUnit.SECONDS); + return null; + }).when(this.simpleDocumentCache).initializeCache(any()); + + DocumentRequiredRights documentRequiredRights = initializeMockDocument(); + + // Launch a background thread so we can test blocking in the initialization of the cache. + ExecutorService executor = Executors.newFixedThreadPool(1); + + try { + CompletableFuture> firstFuture = CompletableFuture.supplyAsync(() -> { + try { + return this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE); + } catch (AuthorizationException e) { + throw new RuntimeException(e); + } + }, executor); + + arrivedInInitializeCacheFuture.get(20, TimeUnit.SECONDS); + + // Ensure that we can still get required rights while the initialization is running. + assertSame(documentRequiredRights, + this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE).orElseThrow()); + + blockInitializeCacheFuture.complete(null); + + assertSame(documentRequiredRights, firstFuture.get(20, TimeUnit.SECONDS).orElseThrow()); + + // Ensure if we call it again, it'll call the cache again. + assertSame(documentRequiredRights, + this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE).orElseThrow()); + + verify(this.simpleDocumentCache, times(2)).get(any(), any()); + verify(this.simpleDocumentCache).initializeCache(any()); + } finally { + executor.shutdownNow(); + } + } + + @Test + void recursiveLoad() throws Exception + { + DocumentRequiredRights documentRequiredRights = initializeMockDocument(); + + Mutable recursiveRights = new MutableObject<>(); + + doAnswer(invocation -> { + recursiveRights.setValue( + this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE).orElseThrow()); + return null; + }).when(this.simpleDocumentCache).initializeCache(any()); + + assertEquals(documentRequiredRights, + this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE).orElseThrow()); + assertEquals(documentRequiredRights, recursiveRights.getValue()); + + verify(this.simpleDocumentCache).initializeCache(any()); + verify(this.simpleDocumentCache).get(eq(DOCUMENT_REFERENCE), any()); + + // Check that loading the required rights again only calls the get() method, and doesn't initialize the cache + // again. + assertEquals(documentRequiredRights, + this.documentRequiredRightsManager.getRequiredRights(DOCUMENT_REFERENCE).orElseThrow()); + + verify(this.simpleDocumentCache).initializeCache(any()); + verify(this.simpleDocumentCache, times(2)).get(eq(DOCUMENT_REFERENCE), any()); + } + + private DocumentRequiredRights initializeMockDocument() throws XWikiException + { + XWikiDocument document = + this.mockitoOldcore.getSpyXWiki().getDocument(DOCUMENT_REFERENCE, this.mockitoOldcore.getXWikiContext()); + this.mockitoOldcore.getSpyXWiki().saveDocument(document, this.mockitoOldcore.getXWikiContext()); + + DocumentRequiredRights documentRequiredRights = mock(); + when(this.documentRequiredRightsReader.readRequiredRights(document)).thenReturn(documentRequiredRights); + return documentRequiredRights; + } +} diff --git a/xwiki-platform-core/xwiki-platform-test/xwiki-platform-test-page/src/main/java/org/xwiki/test/page/PageComponentList.java b/xwiki-platform-core/xwiki-platform-test/xwiki-platform-test-page/src/main/java/org/xwiki/test/page/PageComponentList.java index 3f147ad827be..6f24a5119b4c 100644 --- a/xwiki-platform-core/xwiki-platform-test/xwiki-platform-test-page/src/main/java/org/xwiki/test/page/PageComponentList.java +++ b/xwiki-platform-core/xwiki-platform-test/xwiki-platform-test-page/src/main/java/org/xwiki/test/page/PageComponentList.java @@ -38,7 +38,6 @@ import org.xwiki.display.internal.DocumentContentAsyncRenderer; import org.xwiki.display.internal.DocumentContentDisplayer; import org.xwiki.display.internal.DocumentTitleDisplayer; -import org.xwiki.internal.document.DefaultDocumentRequiredRightsManager; import org.xwiki.internal.document.DocumentRequiredRightsReader; import org.xwiki.internal.script.XWikiScriptContextInitializer; import org.xwiki.internal.velocity.XWikiVelocityManager; @@ -312,7 +311,6 @@ DefaultCacheControl.class, // Required rights (needed for Document/Object API) - DefaultDocumentRequiredRightsManager.class, DocumentRequiredRightsReader.class }) @Inherited