Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add handling for system namespace to CheckedDistributedStorageAdmin #1359

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,33 @@

import com.scalar.db.api.DistributedStorageAdmin;
import com.scalar.db.api.TableMetadata;
import com.scalar.db.config.DatabaseConfig;
import com.scalar.db.exception.storage.ExecutionException;
import com.scalar.db.io.DataType;
import com.scalar.db.util.ScalarDbUtils;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;

public class CheckedDistributedStorageAdmin implements DistributedStorageAdmin {

private final DistributedStorageAdmin admin;
private final String systemNamespaceName;

@SuppressFBWarnings("EI_EXPOSE_REP2")
public CheckedDistributedStorageAdmin(DistributedStorageAdmin admin) {
public CheckedDistributedStorageAdmin(DistributedStorageAdmin admin, DatabaseConfig config) {
this.admin = admin;
systemNamespaceName = config.getSystemNamespaceName();
}

@Override
public void createNamespace(String namespace, Map<String, String> options)
throws ExecutionException {
if (systemNamespaceName.equals(namespace)) {
throw new IllegalArgumentException(namespace + " is the system namespace name");
}
if (namespaceExists(namespace)) {
throw new IllegalArgumentException("Namespace already exists: " + namespace);
}
Expand Down Expand Up @@ -70,6 +77,9 @@ public void dropTable(String namespace, String table) throws ExecutionException

@Override
public void dropNamespace(String namespace) throws ExecutionException {
if (systemNamespaceName.equals(namespace)) {
throw new IllegalArgumentException(namespace + " is the system namespace name");
}
if (!namespaceExists(namespace)) {
throw new IllegalArgumentException("Namespace does not exist: " + namespace);
}
Expand Down Expand Up @@ -189,6 +199,9 @@ public Set<String> getNamespaceTableNames(String namespace) throws ExecutionExce

@Override
public boolean namespaceExists(String namespace) throws ExecutionException {
if (systemNamespaceName.equals(namespace)) {
return true;
}
try {
return admin.namespaceExists(namespace);
} catch (ExecutionException e) {
Expand Down Expand Up @@ -278,7 +291,14 @@ public void addNewColumnToTable(
@Override
public Set<String> getNamespaceNames() throws ExecutionException {
try {
return admin.getNamespaceNames();
Set<String> namespaceNames = admin.getNamespaceNames();
if (namespaceNames.contains(systemNamespaceName)) {
return namespaceNames;
}

namespaceNames = new HashSet<>(namespaceNames);
namespaceNames.add(systemNamespaceName);
return namespaceNames;
} catch (ExecutionException e) {
throw new ExecutionException("Getting the namespace names failed", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ public DistributedStorage createDistributedStorage(DatabaseConfig config) {

@Override
public DistributedStorageAdmin createDistributedStorageAdmin(DatabaseConfig config) {
return new CheckedDistributedStorageAdmin(new CassandraAdmin(config));
return new CheckedDistributedStorageAdmin(new CassandraAdmin(config), config);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ public DistributedStorage createDistributedStorage(DatabaseConfig config) {

@Override
public DistributedStorageAdmin createDistributedStorageAdmin(DatabaseConfig config) {
return new CheckedDistributedStorageAdmin(new CosmosAdmin(config));
return new CheckedDistributedStorageAdmin(new CosmosAdmin(config), config);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ public DistributedStorage createDistributedStorage(DatabaseConfig config) {

@Override
public DistributedStorageAdmin createDistributedStorageAdmin(DatabaseConfig config) {
// Set the namespace check to false because DynamoDB does not support namespaces.
return new CheckedDistributedStorageAdmin(new DynamoAdmin(config));
return new CheckedDistributedStorageAdmin(new DynamoAdmin(config), config);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ public DistributedStorage createDistributedStorage(DatabaseConfig config) {

@Override
public DistributedStorageAdmin createDistributedStorageAdmin(DatabaseConfig config) {
return new CheckedDistributedStorageAdmin(new JdbcAdmin(config));
return new CheckedDistributedStorageAdmin(new JdbcAdmin(config), config);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class JdbcTransactionAdmin implements DistributedTransactionAdmin {

@Inject
public JdbcTransactionAdmin(DatabaseConfig databaseConfig) {
jdbcAdmin = new CheckedDistributedStorageAdmin(new JdbcAdmin(databaseConfig));
jdbcAdmin = new CheckedDistributedStorageAdmin(new JdbcAdmin(databaseConfig), databaseConfig);
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package com.scalar.db.common;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.when;

import com.scalar.db.api.DistributedStorageAdmin;
import com.scalar.db.config.DatabaseConfig;
import com.scalar.db.exception.storage.ExecutionException;
import java.util.Collections;
import java.util.Set;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

public class CheckedDistributedStorageAdminTest {

private static final String SYSTEM_NAMESPACE = "scalardb";

@Mock private DistributedStorageAdmin admin;
@Mock private DatabaseConfig databaseConfig;

private CheckedDistributedStorageAdmin checkedAdmin;

@BeforeEach
public void setUp() throws Exception {
MockitoAnnotations.openMocks(this).close();

// Arrange
when(databaseConfig.getSystemNamespaceName()).thenReturn(SYSTEM_NAMESPACE);
checkedAdmin = new CheckedDistributedStorageAdmin(admin, databaseConfig);
}

@Test
public void createNamespace_SystemNamespaceNameGiven_ShouldThrowIllegalArgumentException() {
// Arrange

// Act Assert
assertThatThrownBy(() -> checkedAdmin.createNamespace(SYSTEM_NAMESPACE))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
public void dropNamespace_SystemNamespaceNameGiven_ShouldThrowIllegalArgumentException() {
// Arrange

// Act Assert
assertThatThrownBy(() -> checkedAdmin.dropNamespace(SYSTEM_NAMESPACE))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
public void namespaceExists_SystemNamespaceNameGiven_ShouldReturnTrue()
throws ExecutionException {
// Arrange

// Act
boolean actual = checkedAdmin.namespaceExists(SYSTEM_NAMESPACE);

// Assert
assertThat(actual).isTrue();
}

@Test
public void getNamespaceNames_ShouldReturnListWithSystemNamespaceName()
throws ExecutionException {
// Arrange
when(admin.getNamespaceNames()).thenReturn(Collections.emptySet());

// Act
Set<String> actual = checkedAdmin.getNamespaceNames();

// Assert
assertThat(actual).containsExactly(SYSTEM_NAMESPACE);
}
}