Skip to content
Closed
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 @@ -32,7 +32,9 @@
import com.exonum.binding.core.storage.indices.StorageIndex;
import com.exonum.binding.core.storage.indices.ValueSetIndexProxy;
import java.util.Optional;
import java.util.OptionalLong;
import java.util.function.Supplier;
import javax.annotation.Nullable;

/**
* Represents an access to the database.
Expand All @@ -48,7 +50,8 @@
*/
public abstract class AbstractAccess extends AbstractNativeProxy implements Access {

private final OpenIndexRegistry indexRegistry = new OpenIndexRegistry();
private static final long UNKNOWN_INDEX_ID = 0L;
private final OpenIndexRegistry indexRegistry;
private final boolean canModify;

/**
Expand All @@ -58,8 +61,20 @@ public abstract class AbstractAccess extends AbstractNativeProxy implements Acce
* @param canModify if the access allows modifications
*/
AbstractAccess(NativeHandle nativeHandle, boolean canModify) {
this(nativeHandle, canModify, new OpenIndexRegistry());
}

/**
* Create a new access proxy with the given index registry.
*
* @param nativeHandle a native handle: an implementation-specific reference to a native object
* @param canModify if the access allows modifications
* @param registry a pool of open indexes to use
*/
AbstractAccess(NativeHandle nativeHandle, boolean canModify, OpenIndexRegistry registry) {
super(nativeHandle);
this.canModify = canModify;
indexRegistry = registry;
}

@SuppressWarnings("unchecked") // The compiler is correct: the cache is not type-safe: ECR-3387
Expand Down Expand Up @@ -140,8 +155,15 @@ private <T extends StorageIndex> T findOrCreate(IndexAddress address, Class<T> i
*/
private <T extends StorageIndex> Optional<T> findOpenIndex(IndexAddress address,
Class<T> indexType) {
return indexRegistry.findIndex(address)
.map(index -> checkedCast(index, indexType));
OptionalLong indexId = findIndexId(address);
if (indexId.isPresent()) {
// An index with the given address exists in the storage
return indexRegistry.findIndex(indexId.getAsLong())
.map(index -> checkedCast(index, indexType));
} else {
// The index does not exist; hence cannot be in the cache
return Optional.empty();
}
}

/**
Expand All @@ -161,16 +183,44 @@ private static <IndexT extends StorageIndex> IndexT checkedCast(StorageIndex cac
}

private <T extends StorageIndex> T createIndex(Supplier<T> indexSupplier) {
// Create an index
T newIndex = indexSupplier.get();
registerIndex(newIndex);
// Find the id of the index (possibly, newly created)
OptionalLong indexId = findIndexId(newIndex.getAddress());
// Register the open index in the pool, if it exists.
// It does not "exist" until it is created with a Fork-based Access,
// i.e., an empty index created with the Snapshot will not have an id.
// We can, of course, cache them by address then, but that is not
// required for correctness and may be done later as an optimization.
indexId.ifPresent(id -> registerIndex(id, newIndex));
return newIndex;
}

private OptionalLong findIndexId(IndexAddress address) {
long id = findIndexId(address.getName(), address.getIdInGroup().orElse(null));
if (id == UNKNOWN_INDEX_ID) {
return OptionalLong.empty();
}
return OptionalLong.of(id);
}

/**
* Finds an internal unique MerkleDB id of an index with the given relative name
* and optional id in group.
* @return a non-zero id if the index is created in the database; zero if the index
* does not exist
*/
/* TODO: either native — if all Accesses can have same impl; or abstract and native
in each Access [ECR-4157] */
private long findIndexId(String name, @Nullable byte[] idInGroup) {
return 0;
}

/**
* Registers a new index created with this access.
*/
private void registerIndex(StorageIndex index) {
indexRegistry.registerIndex(index);
private void registerIndex(Long id, StorageIndex index) {
indexRegistry.registerIndex(id, index);
}

/**
Expand All @@ -193,6 +243,13 @@ public long getAccessNativeHandle() {
return super.getNativeHandle();
}

/**
* Returns the registry of open indexes for this Access.
*/
protected OpenIndexRegistry getOpenIndexes() {
return indexRegistry;
}

/**
* Returns the cleaner of this access. It is supposed to be used with collections
* and other objects depending on this access.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import static com.google.common.base.Preconditions.checkArgument;

import com.exonum.binding.core.storage.indices.IndexAddress;
import com.exonum.binding.core.storage.indices.StorageIndex;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -29,20 +28,21 @@
* to de-duplicate the indexes created with the same (Access, name, prefix) tuple, which is
* required to overcome the MerkleDB limitation which prevents creating several indexes
* with the same address (name + prefix) using the same Fork.
*
* <p>See {@code IndexMetadata} and {@code Access.get_index_metadata} in Rust.
*/
class OpenIndexRegistry {

private final Map<IndexAddress, StorageIndex> indexes = new HashMap<>();
private final Map<Long, StorageIndex> indexes = new HashMap<>();

void registerIndex(StorageIndex index) {
IndexAddress address = index.getAddress();
Object present = indexes.putIfAbsent(address, index);
checkArgument(present == null, "Cannot register index (%s): the address (%s) is already "
+ "associated with index (%s): ", index, address, present);
void registerIndex(Long id, StorageIndex index) {
Object present = indexes.putIfAbsent(id, index);
checkArgument(present == null, "Cannot register index (%s): the id (%s) is already "
+ "associated with index (%s): ", index, id, present);
}

Optional<StorageIndex> findIndex(IndexAddress address) {
return Optional.ofNullable(indexes.get(address));
Optional<StorageIndex> findIndex(Long id) {
return Optional.ofNullable(indexes.get(id));
}

void clear() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* Copyright 2020 The Exonum Team
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.exonum.binding.core.storage.database;

import static com.google.common.base.Preconditions.checkNotNull;

import com.exonum.binding.core.proxy.Cleaner;
import com.exonum.binding.core.proxy.NativeHandle;
import com.exonum.binding.core.proxy.ProxyDestructor;
import com.exonum.binding.core.storage.indices.IndexAddress;
import com.exonum.binding.core.util.LibraryLoader;
import com.google.common.annotations.VisibleForTesting;

/**
* A prefixed database access. It uses a base Access, and adds an address resolution.
*
* <p>The Prefixed Access resolves the index addresses by prepending a namespace, followed by
* a dot ('.'), to the {@linkplain IndexAddress#getName() name part} of the address.
*
* <p>This class is a native proxy of the {@code Prefixed} Rust Access.
*/
public final class Prefixed extends AbstractAccess {

static {
LibraryLoader.load();
}

private final Cleaner cleaner;

Prefixed(NativeHandle prefixedNativeHandle, boolean canModify, Cleaner cleaner,
OpenIndexRegistry registry) {
super(prefixedNativeHandle, canModify, registry);
this.cleaner = cleaner;
}

/**
* Creates a new Prefixed access given the base database access and the namespace.
*
* <p>The new Prefixed access will inherit from the base access its "canModify" property;
* and use its cleaner and registry of open indexes. When that cleaner gets closed,
* this access will also be closed.
*
* @param namespace the namespace to use
* @param baseAccess the base database access
*/
@VisibleForTesting static Prefixed fromAccess(String namespace, AbstractAccess baseAccess) {
// todo: If it is not used beyond tests, consider removing the _registry sharing_,
// since that's the only method that uses (and needs) shared index registry.
Cleaner cleaner = baseAccess.getCleaner();
OpenIndexRegistry registry = baseAccess.getOpenIndexes();
long handle = nativeCreate(namespace, baseAccess.getAccessNativeHandle());
return fromHandleInternal(handle, baseAccess.canModify(), cleaner, registry);
}

/**
* Creates a Prefixed native peer given the base access native handle.
* @throws RuntimeException if the base access is unsupported; or if the namespace is not valid
*/
private static native long nativeCreate(String namespace, long accessNativeHandle);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we usually use (handle, arguments....) order in native methods

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In create* a handle is just an argument, it is not a handle to "this".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I guess it's the matter of taste


/**
* Creates a new Prefixed access from the native handle. The destructor will be registered
* in the given cleaner.
*
* @param prefixedNativeHandle a handle to the native Prefixed Access
* @param canModify whether the base access allows modifications
* @param cleaner a cleaner to destroy the native peer and any dependent objects
*/
public static Prefixed fromHandle(long prefixedNativeHandle, boolean canModify, Cleaner cleaner) {
checkNotNull(cleaner);
// When the base Access is unknown (hidden in native) — use a separate pool of open indexes
OpenIndexRegistry registry = new OpenIndexRegistry();
return fromHandleInternal(prefixedNativeHandle, canModify, cleaner, registry);
}

/**
* Expects validated parameters so that it does not throw and registers the destructor
* properly, which is *required* to prevent leaks.
*/
private static Prefixed fromHandleInternal(long prefixedNativeHandle, boolean canModify,
Cleaner cleaner, OpenIndexRegistry registry) {
NativeHandle handle = new NativeHandle(prefixedNativeHandle);
ProxyDestructor.newRegistered(cleaner, handle, Prefixed.class,
/* todo [ECR-4161]: Verify after native implementation! */ Accesses::nativeFree);
return new Prefixed(handle, canModify, cleaner, registry);
}

@Override
public Cleaner getCleaner() {
return cleaner;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.exonum.binding.core.storage.indices.StoragePreconditions.checkIdInGroup;
import static com.exonum.binding.core.storage.indices.StoragePreconditions.checkIndexName;

import com.exonum.binding.core.storage.database.Access;
import com.google.common.base.MoreObjects;
import java.util.Arrays;
import java.util.Objects;
Expand All @@ -29,6 +30,9 @@
* An Exonum index address: a pair of the name and an optional id in a group, which identifies
* an Exonum index.
*
* <p>Index addresses are resolved relatively to database {@link Access} objects.
* An index address cannot be translated into a resolved address without the corresponding
* {@link Access} object.
* <!-- todo: Extend given accesses — shall we keep just IndexAddress, or split into Relative
* and Resolved IndexAddress? -->
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,25 @@
*/
public interface StorageIndex {

/** Returns the name of this index. */
/**
* Returns the name of this index.
*
* <p>Please note that the implementations may return either relative or absolute name.
* The name is not required to be equal to the one passed to the index constructor.
* <!-- This vague-ish clause and strange behaviour are needed to allow index caching across
* Accesses -->
*/
default String getName() {
return getAddress().getName();
}

/**
* Returns the <em>index address</em>: its unique identifier in the database. It consists
* of the name and, in case this index belongs to an index family, a family identifier.
* Returns the <em>index address</em>: its identifier in the database.
*
* <p>Please note that the implementations may return either relative or absolute address.
* The address is not required to be equal to the one passed to the index constructor.
* <!-- This vague-ish clause and strange behaviour are needed to allow index caching across
* Accesses. -->
*/
IndexAddress getAddress();
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import com.exonum.binding.core.storage.indices.IndexAddress;
import com.exonum.binding.core.storage.indices.StorageIndex;
import java.util.Optional;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -35,33 +33,30 @@ class OpenIndexRegistryTest {
@Nested
class WithSingleIndex {

private final IndexAddress address = IndexAddress.valueOf("name");
private final Long id = 1L;
private final StorageIndex index = mock(StorageIndex.class, "index 1");

@BeforeEach
void registerIndex() {
when(index.getAddress()).thenReturn(address);

registry.registerIndex(index);
registry.registerIndex(id, index);
}

@Test
void canFindRegisteredIndex() {
Optional<StorageIndex> actual = registry.findIndex(address);
Optional<StorageIndex> actual = registry.findIndex(id);

assertThat(actual).hasValue(index);
}

@Test
void registerThrowsIfAlreadyRegisteredSameAddress() {
void registerThrowsIfAlreadyRegisteredSameId() {
StorageIndex otherIndex = mock(StorageIndex.class, "other index");
when(otherIndex.getAddress()).thenReturn(address);

Exception e = assertThrows(IllegalArgumentException.class,
() -> registry.registerIndex(otherIndex));
() -> registry.registerIndex(id, otherIndex));

String message = e.getMessage();
assertThat(message).contains(String.valueOf(address))
assertThat(message).contains(String.valueOf(id))
.contains(String.valueOf(index))
.contains(String.valueOf(otherIndex));
}
Expand All @@ -70,16 +65,16 @@ void registerThrowsIfAlreadyRegisteredSameAddress() {
void clearRemovesTheIndex() {
registry.clear();

Optional<StorageIndex> actual = registry.findIndex(address);
Optional<StorageIndex> actual = registry.findIndex(id);

assertThat(actual).isEmpty();
}
}

@Test
void findUnknownIndex() {
IndexAddress unknownAddress = IndexAddress.valueOf("Unknown");
Optional<StorageIndex> index = registry.findIndex(unknownAddress);
long unknownId = 1024L;
Optional<StorageIndex> index = registry.findIndex(unknownId);

assertThat(index).isEmpty();
}
Expand Down
Loading