Nemcio/SCM-Manager
1
0
Fork 0
mirror of https://github.com/scm-manager/scm-manager.git synced 2025-10-26 00:56:09 +02:00
Code Issues Packages Projects Releases Activity

Implement read-only check for queryable stores

Browse Source 
Squash commits of branch feature/write_protected_queryable_store:

- Bootstrap read-only check for queryable stores

- Use class name instead of static string

- Fix build breaker

- Log change

- Add unit tests

- Clean up

- Merge branch 'develop' into feature/write_protected_queryable_store
This commit is contained in:
Rene Pfeuffer
2025-06-19 16:19:45 +02:00
parent 4b49748561
commit 7c9aa016d8
7 changed files with 172 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
gradle/changelog/write_lock_check.yaml Normal file
View File

@@ -0,0 +1,2 @@
- type: added
description: Write lock check for queryable stores

5
scm-persistence/src/main/java/sonia/scm/store/sqlite/SQLiteQueryableMutableStore.java
View File

@@ -55,8 +55,9 @@ class SQLiteQueryableMutableStore<T> extends SQLiteQueryableStore<T> implements
Class<T> clazz,
QueryableTypeDescriptor queryableTypeDescriptor,
String[] parentIds,
ReadWriteLock lock) {
super(objectMapper, connection, clazz, queryableTypeDescriptor, parentIds, lock);
ReadWriteLock lock,
boolean readOnly) {
super(objectMapper, connection, clazz, queryableTypeDescriptor, parentIds, lock, readOnly);
this.objectMapper = objectMapper;
this.clazz = clazz;
this.parentIds = parentIds;

13
scm-persistence/src/main/java/sonia/scm/store/sqlite/SQLiteQueryableStore.java
View File

@@ -30,6 +30,7 @@ import sonia.scm.store.LogicalCondition;
import sonia.scm.store.QueryableMaintenanceStore;
import sonia.scm.store.QueryableStore;
import sonia.scm.store.StoreException;
import sonia.scm.store.StoreReadOnlyException;
import java.sql.Connection;
import java.sql.PreparedStatement;
@@ -71,19 +72,22 @@ class SQLiteQueryableStore<T> implements QueryableStore<T>, QueryableMaintenance
private final String[] parentIds;
private final ReadWriteLock lock;
private final boolean readOnly;
public SQLiteQueryableStore(ObjectMapper objectMapper,
Connection connection,
Class<T> clazz,
QueryableTypeDescriptor queryableTypeDescriptor,
String[] parentIds,
ReadWriteLock lock) {
ReadWriteLock lock,
boolean readOnly) {
this.objectMapper = objectMapper;
this.connection = connection;
this.clazz = clazz;
this.parentIds = parentIds;
this.queryableTypeDescriptor = queryableTypeDescriptor;
this.lock = lock;
this.readOnly = readOnly;
}
@Override
@@ -243,11 +247,18 @@ class SQLiteQueryableStore<T> implements QueryableStore<T>, QueryableMaintenance
}
<R> R executeWrite(SQLNodeWithValue sqlStatement, StatementCallback<R> callback) {
assertNotReadOnly();
String sql = sqlStatement.toSQL();
log.debug("executing 'write' SQL: {}", sql);
return executeWithLock(sqlStatement, callback, lock.writeLock(), sql);
}
private void assertNotReadOnly() {
if (readOnly) {
throw new StoreReadOnlyException(clazz.getName());
}
}
private <R> R executeWithLock(SQLNodeWithValue sqlStatement, StatementCallback<R> callback, Lock writeLock, String sql) {
writeLock.lock();
try (PreparedStatement statement = connection.prepareStatement(sql, RETURN_GENERATED_KEYS)) {

58
scm-persistence/src/main/java/sonia/scm/store/sqlite/SQLiteQueryableStoreFactory.java
View File

@@ -28,6 +28,8 @@ import sonia.scm.SCMContextProvider;
import sonia.scm.config.ConfigValue;
import sonia.scm.plugin.PluginLoader;
import sonia.scm.plugin.QueryableTypeDescriptor;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryReadOnlyChecker;
import sonia.scm.security.KeyGenerator;
import sonia.scm.store.QueryableMaintenanceStore;
import sonia.scm.store.QueryableStoreFactory;
@@ -59,6 +61,7 @@ public class SQLiteQueryableStoreFactory implements QueryableStoreFactory {
private final ObjectMapper objectMapper;
private final KeyGenerator keyGenerator;
private final DataSource dataSource;
private final RepositoryReadOnlyChecker readOnlyChecker;
private final Map<String, QueryableTypeDescriptor> queryableTypes = new HashMap<>();
@@ -69,6 +72,7 @@ public class SQLiteQueryableStoreFactory implements QueryableStoreFactory {
PluginLoader pluginLoader,
ObjectMapper objectMapper,
KeyGenerator keyGenerator,
RepositoryReadOnlyChecker readOnlyChecker,
@ConfigValue(key = "queryableStore.maxPoolSize", defaultValue = DEFAULT_MAX_POOL_SIZE) int maxPoolSize,
@ConfigValue(key = "queryableStore.connectionTimeout", defaultValue = DEFAULT_CONNECTION_TIMEOUT_IN_SECONDS) int connectionTimeoutInSeconds,
@ConfigValue(key = "queryableStore.idleTimeout", defaultValue = DEFAULT_IDLE_TIMEOUT_IN_SECONDS) int idleTimeoutInSeconds,
@@ -80,6 +84,7 @@ public class SQLiteQueryableStoreFactory implements QueryableStoreFactory {
objectMapper,
keyGenerator,
pluginLoader.getExtensionProcessor().getQueryableTypes(),
readOnlyChecker,
maxPoolSize,
connectionTimeoutInSeconds,
idleTimeoutInSeconds,
@@ -92,14 +97,16 @@ public class SQLiteQueryableStoreFactory implements QueryableStoreFactory {
public SQLiteQueryableStoreFactory(String connectionString,
ObjectMapper objectMapper,
KeyGenerator keyGenerator,
Iterable<QueryableTypeDescriptor> queryableTypeIterable) {
this(connectionString, objectMapper, keyGenerator, queryableTypeIterable, 10, 30, 600, 1800, 30);
Iterable<QueryableTypeDescriptor> queryableTypeIterable,
RepositoryReadOnlyChecker readOnlyChecker) {
this(connectionString, objectMapper, keyGenerator, queryableTypeIterable, readOnlyChecker, 10, 30, 600, 1800, 30);
}
private SQLiteQueryableStoreFactory(String connectionString,
ObjectMapper objectMapper,
KeyGenerator keyGenerator,
Iterable<QueryableTypeDescriptor> queryableTypeIterable,
RepositoryReadOnlyChecker readOnlyChecker,
int maxPoolSize,
int connectionTimeoutInSeconds,
int idleTimeoutInSeconds,
@@ -112,6 +119,7 @@ public class SQLiteQueryableStoreFactory implements QueryableStoreFactory {
idleTimeoutInSeconds,
maxLifetimeInSeconds,
leakDetectionThresholdInSeconds);
this.readOnlyChecker = readOnlyChecker;
this.dataSource = new HikariDataSource(config);
this.objectMapper = objectMapper
@@ -170,17 +178,57 @@ public class SQLiteQueryableStoreFactory implements QueryableStoreFactory {
@Override
public <T> SQLiteQueryableStore<T> getReadOnly(Class<T> clazz, String... parentIds) {
return new SQLiteQueryableStore<>(objectMapper, openDefaultConnection(), clazz, getQueryableTypeDescriptor(clazz), parentIds, lock);
QueryableTypeDescriptor queryableTypeDescriptor = getQueryableTypeDescriptor(clazz);
return new SQLiteQueryableStore<>(
objectMapper,
openDefaultConnection(),
clazz,
queryableTypeDescriptor,
parentIds,
lock,
mustBeReadOnly(queryableTypeDescriptor, parentIds)
);
}
@Override
public <T> QueryableMaintenanceStore<T> getForMaintenance(Class<T> clazz, String... parentIds) {
return new SQLiteQueryableStore<>(objectMapper, openDefaultConnection(), clazz, getQueryableTypeDescriptor(clazz), parentIds, lock);
QueryableTypeDescriptor queryableTypeDescriptor = getQueryableTypeDescriptor(clazz);
return new SQLiteQueryableStore<>(
objectMapper,
openDefaultConnection(),
clazz,
queryableTypeDescriptor,
parentIds,
lock,
mustBeReadOnly(queryableTypeDescriptor, parentIds)
);
}
@Override
public <T> SQLiteQueryableMutableStore<T> getMutable(Class<T> clazz, String... parentIds) {
return new SQLiteQueryableMutableStore<>(objectMapper, keyGenerator, openDefaultConnection(), clazz, getQueryableTypeDescriptor(clazz), parentIds, lock);
QueryableTypeDescriptor queryableTypeDescriptor = getQueryableTypeDescriptor(clazz);
return new SQLiteQueryableMutableStore<>(
objectMapper,
keyGenerator,
openDefaultConnection(),
clazz,
queryableTypeDescriptor,
parentIds,
lock,
mustBeReadOnly(queryableTypeDescriptor, parentIds)
);
}
private boolean mustBeReadOnly(QueryableTypeDescriptor queryableTypeDescriptor, String... parentIds) {
for (int i = 0; i < queryableTypeDescriptor.getTypes().length; i++) {
if (queryableTypeDescriptor.getTypes()[i].startsWith(Repository.class.getName()) && parentIds.length > i) {
String repositoryId = parentIds[i];
if (repositoryId != null && readOnlyChecker.isReadOnly(repositoryId)) {
return true;
}
}
}
return false;
}
private <T> QueryableTypeDescriptor getQueryableTypeDescriptor(Class<T> clazz) {

73
scm-persistence/src/test/java/sonia/scm/store/sqlite/SQLiteQueryableMutableStoreTest.java
View File

@@ -21,9 +21,12 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import sonia.scm.group.Group;
import sonia.scm.repository.Repository;
import sonia.scm.store.IdGenerator;
import sonia.scm.store.QueryableMutableStore;
import sonia.scm.store.QueryableStore;
import sonia.scm.store.StoreReadOnlyException;
import sonia.scm.user.User;
import java.nio.file.Path;
@@ -36,6 +39,7 @@ import java.util.Map;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertThrows;
@SuppressWarnings("resource")
class SQLiteQueryableMutableStoreTest {
@@ -510,6 +514,75 @@ class SQLiteQueryableMutableStoreTest {
assertThat(crewmateStoreForShipTwo.getAll()).hasSize(2);
}
}
}
@Test
void shouldThrowExceptionWhenReadOnlyStoreIsModified() {
StoreTestBuilder storeTestBuilder = new StoreTestBuilder(
connectionString,
Repository.class.getName() + ".class"
).readOnly("42");
assertThrows(StoreReadOnlyException.class, () ->
storeTestBuilder
.withIds("42")
.put("tricia", new User("trillian"))
);
}
@Test
void shouldThrowExceptionWhenStoreForReadOnlyRepositoryAsFirstParentIsModified() {
StoreTestBuilder storeTestBuilder = new StoreTestBuilder(
connectionString,
Repository.class.getName() + ".class",
Group.class.getName() + ".class"
).readOnly("42");
assertThrows(StoreReadOnlyException.class, () ->
storeTestBuilder
.withIds("42", "hitchhikers")
.put("tricia", new User("trillian"))
);
}
@Test
void shouldThrowExceptionWhenStoreForReadOnlyRepositoryAsSecondParentIsModified() {
StoreTestBuilder storeTestBuilder = new StoreTestBuilder(
connectionString,
Group.class.getName() + ".class",
Repository.class.getName() + ".class"
).readOnly("42");
assertThrows(StoreReadOnlyException.class, () ->
storeTestBuilder
.withIds("hitchhikers", "42")
.put("tricia", new User("trillian"))
);
}
@Test
void shouldWriteToWritableStore() {
StoreTestBuilder storeTestBuilder = new StoreTestBuilder(
connectionString,
Repository.class.getName() + ".class"
).readOnly("42");
SQLiteQueryableMutableStore<User> store = storeTestBuilder.withIds("23");
store.put("tricia", new User("trillian"));
assertThat(store.get("tricia")).isNotNull();
}
@Test
void shouldWriteToWritableStoreWithDifferentParentClass() {
StoreTestBuilder storeTestBuilder = new StoreTestBuilder(
connectionString,
Group.class.getName() + ".class"
).readOnly("42");
SQLiteQueryableMutableStore<User> store = storeTestBuilder.withIds("42");
store.put("tricia", new User("trillian"));
assertThat(store.get("tricia")).isNotNull();
}
}

27
scm-persistence/src/test/java/sonia/scm/store/sqlite/StoreTestBuilder.java
View File

@@ -18,17 +18,25 @@ package sonia.scm.store.sqlite;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import org.mockito.Mockito;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryReadOnlyChecker;
import sonia.scm.security.UUIDKeyGenerator;
import sonia.scm.store.IdGenerator;
import sonia.scm.store.QueryableMaintenanceStore;
import sonia.scm.store.QueryableStore;
import sonia.scm.user.User;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES;
import static com.fasterxml.jackson.databind.SerializationFeature.WRITE_DATES_AS_TIMESTAMPS;
import static com.fasterxml.jackson.databind.SerializationFeature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.Mockito.when;
import static sonia.scm.store.sqlite.QueryableTypeDescriptorTestData.createDescriptor;
class StoreTestBuilder {
@@ -48,6 +56,8 @@ class StoreTestBuilder {
private final String[] parentClasses;
private final IdGenerator idGenerator;
private Collection<String> readOnlyRepositoryIds = new HashSet<>();
StoreTestBuilder(String connectionString, String... parentClasses) {
this(connectionString, IdGenerator.DEFAULT, parentClasses);
}
@@ -80,12 +90,27 @@ class StoreTestBuilder {
return createStoreFactory(clazz).getMutable(clazz, ids);
}
StoreTestBuilder readOnly(String... repositoryIds) {
readOnlyRepositoryIds.addAll(List.of(repositoryIds));
return this;
}
private <T> SQLiteQueryableStoreFactory createStoreFactory(Class<T> clazz) {
RepositoryReadOnlyChecker readOnlyChecker;
if (readOnlyRepositoryIds.isEmpty()) {
readOnlyChecker = new RepositoryReadOnlyChecker();
} else {
readOnlyChecker = Mockito.mock(RepositoryReadOnlyChecker.class);
when(readOnlyChecker.isReadOnly(argThat((String repoId) -> readOnlyRepositoryIds.contains(repoId))))
.thenReturn(true);
when(readOnlyChecker.isReadOnly(any(Repository.class))).thenCallRealMethod();
}
return new SQLiteQueryableStoreFactory(
connectionString,
mapper,
new UUIDKeyGenerator(),
List.of(createDescriptor("", clazz.getName(), parentClasses, idGenerator))
List.of(createDescriptor("", clazz.getName(), parentClasses, idGenerator)),
readOnlyChecker
);
}
}

4
scm-queryable-test/src/main/java/sonia/scm/store/QueryableStoreExtension.java
View File

@@ -35,6 +35,7 @@ import org.junit.jupiter.api.extension.ParameterContext;
import org.junit.jupiter.api.extension.ParameterResolutionException;
import org.junit.jupiter.api.extension.ParameterResolver;
import sonia.scm.plugin.QueryableTypeDescriptor;
import sonia.scm.repository.RepositoryReadOnlyChecker;
import sonia.scm.security.UUIDKeyGenerator;
import sonia.scm.store.sqlite.SQLiteQueryableStoreFactory;
import sonia.scm.util.IOUtil;
@@ -101,7 +102,8 @@ public class QueryableStoreExtension implements ParameterResolver, BeforeEachCal
connectionString,
mapper,
new UUIDKeyGenerator(),
queryableTypeDescriptors
queryableTypeDescriptors,
new RepositoryReadOnlyChecker()
)
);
}