Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
8ab6ed8
Changes to run tests for multihash collections
SrinikhilReddy Dec 14, 2020
d5d3b47
Changes.
SrinikhilReddy Jan 7, 2021
5459104
Parititionkey builder model as suggested by Mo.
SrinikhilReddy Jan 15, 2021
e1c3001
Apply suggestions from code review
SrinikhilReddy Jan 15, 2021
29f2cea
Merge remote-tracking branch 'upstream/master'
SrinikhilReddy Feb 16, 2021
dd77aaf
Pushing to remote. Cleanup
SrinikhilReddy Feb 16, 2021
a65e716
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java
SrinikhilReddy Feb 16, 2021
58c2746
commit local changes
SrinikhilReddy Apr 6, 2021
4cb8cb1
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java
SrinikhilReddy Apr 6, 2021
036b7ec
Address PR comments.
SrinikhilReddy Apr 6, 2021
f2905f4
Address code comments.
SrinikhilReddy Apr 6, 2021
24610d6
Resolve code comments
SrinikhilReddy Apr 6, 2021
5a741c5
Add tests.
SrinikhilReddy Apr 8, 2021
29bcfc9
Well, when does this stop.
SrinikhilReddy Apr 8, 2021
14bd7a2
No more comments.
SrinikhilReddy Apr 8, 2021
8f77d0d
Update CosmosMultiHashTest.java
SrinikhilReddy May 6, 2021
741ce81
Merge remote-tracking branch 'upstream/master'
SrinikhilReddy May 6, 2021
0b45b66
Fix test failures
SrinikhilReddy May 10, 2021
22fe137
Merge remote-tracking branch 'upstream/master'
SrinikhilReddy May 10, 2021
5bf9d26
Fixing code suggestions
SrinikhilReddy May 11, 2021
0c67ba6
Merge remote-tracking branch 'upstream/master'
SrinikhilReddy May 11, 2021
00d5f8e
Resolve code comments
SrinikhilReddy May 12, 2021
f008411
Merge remote-tracking branch 'upstream/master'
SrinikhilReddy May 12, 2021
28926cf
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java
SrinikhilReddy May 20, 2021
ed325f9
Merge remote-tracking branch 'upstream/master'
SrinikhilReddy Jun 10, 2021
b6e452e
Fixing a test
SrinikhilReddy Jun 10, 2021
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 @@ -261,7 +261,7 @@ public static class A_IMHeaderValues {
}

public static class Versions {
public static final String CURRENT_VERSION = "2018-12-31";
public static final String CURRENT_VERSION = "2020-07-15";
Comment thread
SrinikhilReddy marked this conversation as resolved.
public static final String QUERY_VERSION = "1.0";
public static final String AZURE_COSMOS_PROPERTIES_FILE_NAME = "azure-cosmos.properties";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1222,19 +1222,33 @@ private static PartitionKeyInternal extractPartitionKeyValueFromDocument(
InternalObjectNode document,
PartitionKeyDefinition partitionKeyDefinition) {
if (partitionKeyDefinition != null) {
String path = partitionKeyDefinition.getPaths().iterator().next();
List<String> parts = PathParser.getPathParts(path);
if (parts.size() >= 1) {
Object value = ModelBridgeInternal.getObjectByPathFromJsonSerializable(document, parts);
if (value == null || value.getClass() == ObjectNode.class) {
value = ModelBridgeInternal.getNonePartitionKey(partitionKeyDefinition);
}
switch (partitionKeyDefinition.getKind()) {
Comment thread
SrinikhilReddy marked this conversation as resolved.
case HASH:

String path = partitionKeyDefinition.getPaths().iterator().next();
List<String> parts = PathParser.getPathParts(path);
if (parts.size() >= 1) {
Object value = ModelBridgeInternal.getObjectByPathFromJsonSerializable(document, parts);
if (value == null || value.getClass() == ObjectNode.class) {
value = ModelBridgeInternal.getNonePartitionKey(partitionKeyDefinition);
}

if (value instanceof PartitionKeyInternal) {
return (PartitionKeyInternal) value;
} else {
return PartitionKeyInternal.fromObjectArray(Collections.singletonList(value), false);
if (value instanceof PartitionKeyInternal) {
return (PartitionKeyInternal) value;
} else {
return PartitionKeyInternal.fromObjectArray(Collections.singletonList(value), false);
}
}
break;
case MULTI_HASH:
Object[] partitionKeyValues = new Object[partitionKeyDefinition.getPaths().size()];
for(int path_iter = 0 ; path_iter < partitionKeyDefinition.getPaths().size(); path_iter++)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@moderakh any suggestions on elegant way of iteration.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it is possible to use java stream.

partitionKeyDefinition.getPaths().stream().map ....

but not much difference functional wise.

{
Comment thread
SrinikhilReddy marked this conversation as resolved.
Outdated
String partitionPath = partitionKeyDefinition.getPaths().get(path_iter);
List<String> partitionPathParts = PathParser.getPathParts(partitionPath);
partitionKeyValues[path_iter] = ModelBridgeInternal.getObjectByPathFromJsonSerializable(document, partitionPathParts);
}
return PartitionKeyInternal.fromObjectArray(partitionKeyValues, false);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,32 @@ static public String getEffectivePartitionKeyForHashPartitioningV2(PartitionKeyI
}
}

static String getEffectivePartitionKeyForMultiHashPartitioning(PartitionKeyInternal partitionKeyInternal) {
StringBuilder stringBuilder = new StringBuilder();
Comment thread
SrinikhilReddy marked this conversation as resolved.
Outdated
for (int i = 0; i < partitionKeyInternal.components.size(); i++) {
try(ByteBufferOutputStream byteArrayBuffer = new ByteBufferOutputStream()) {
Comment thread
kirankumarkolli marked this conversation as resolved.

partitionKeyInternal.components.get(i).writeForHashingV2(byteArrayBuffer);


ByteBuffer byteBuffer = byteArrayBuffer.asByteBuffer();
UInt128 hashAsUnit128 = MurmurHash3_128.hash128(byteBuffer.array(), byteBuffer.limit());

byte[] hash = uIntToBytes(hashAsUnit128);
Bytes.reverse(hash);

// Reset 2 most significant bits, as max exclusive value is 'FF'.
// Plus one more just in case.
hash[0] &= 0x3F;

stringBuilder.append(HexConvert.bytesToHex(hash));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Q: How about allocate byte[] and then convert to hex at the end?
Purely from performance wise. .NET micro-benchmark will be useful for quick prototyping.

} catch (IOException e) {
throw new IllegalArgumentException(e);
}
}
return stringBuilder.toString();
Comment thread
SrinikhilReddy marked this conversation as resolved.
}

static String getEffectivePartitionKeyForHashPartitioning(PartitionKeyInternal partitionKeyInternal) {
IPartitionKeyComponent[] truncatedComponents = new IPartitionKeyComponent[partitionKeyInternal.components.size()];

Expand Down Expand Up @@ -138,7 +164,7 @@ public static String getEffectivePartitionKeyString(PartitionKeyInternal partiti
return MaximumExclusiveEffectivePartitionKey;
}

if (partitionKeyInternal.components.size() < partitionKeyDefinition.getPaths().size()) {
if (partitionKeyInternal.components.size() < partitionKeyDefinition.getPaths().size() && partitionKeyDefinition.getKind() != PartitionKind.MULTI_HASH) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this to cover only for query?

If so do we need two variations?
Like what happens for point operations partial pk is specified?

throw new IllegalArgumentException(RMResources.TooFewPartitionKeyComponents);
}

Expand All @@ -161,6 +187,9 @@ public static String getEffectivePartitionKeyString(PartitionKeyInternal partiti
return getEffectivePartitionKeyForHashPartitioning(partitionKeyInternal);
}

case MULTI_HASH:
return getEffectivePartitionKeyForMultiHashPartitioning(partitionKeyInternal);

default:
return toHexEncodedBinaryString(partitionKeyInternal.components);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ public PartitionKey(final Object key) {
this.internalPartitionKey = PartitionKeyInternal.fromObjectArray(new Object[] {key}, true);
}

/**
* Constructor. CREATE a new instance of the PartitionKey object.
*
* @param keys the value of partition keys.
*/
@SuppressWarnings("serial")
public PartitionKey(final Object[] keys) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Object can be any non primitive type, however for subpartitioning we only support primitive types,

we should consider using a builder pattern to construct and validate the content instead of passing raw object array.

Could you check what DotNet v3 is doing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added the pattern as suggested.

this.keyObject = keys;
this.internalPartitionKey = PartitionKeyInternal.fromObjectArray(keys, true);
}

/**
* Gets the object used to create partition key
* @return the partition key object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ public enum PartitionKind {
/**
Comment thread
SrinikhilReddy marked this conversation as resolved.
* The Partition of a item is calculated based on the hash value of the PartitionKey.
*/
HASH("Hash");
HASH("Hash"),

RANGE("Range"),
Comment thread
SrinikhilReddy marked this conversation as resolved.
/**
* The Partition of a item is calculated based on the hash value of multiple PartitionKeys.
*/
MULTI_HASH("MultiHash");
Comment thread
SrinikhilReddy marked this conversation as resolved.

PartitionKind(String overWireValue) {
this.overWireValue = overWireValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,9 @@

package com.azure.cosmos;

import com.azure.cosmos.implementation.Document;
import com.azure.cosmos.implementation.HttpConstants;
import com.azure.cosmos.models.CosmosContainerProperties;
import com.azure.cosmos.models.CosmosContainerRequestOptions;
import com.azure.cosmos.models.CosmosContainerResponse;
import com.azure.cosmos.models.CosmosQueryRequestOptions;
import com.azure.cosmos.models.FeedRange;
import com.azure.cosmos.models.IndexingMode;
import com.azure.cosmos.models.IndexingPolicy;
import com.azure.cosmos.models.SqlQuerySpec;
import com.azure.cosmos.models.ThroughputProperties;
import com.azure.cosmos.models.*;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

code style: we don't do wildcard import.

You can change your intellj/eclipse default behaviour not not use wildcard import.

see this: https://stackoverflow.com/questions/3348816/intellij-never-use-wildcard-imports

import com.azure.cosmos.rx.TestSuiteBase;
import com.azure.cosmos.util.CosmosPagedIterable;
import org.testng.annotations.AfterClass;
Expand All @@ -26,6 +19,7 @@

import java.util.List;
import java.util.UUID;
import java.util.ArrayList;

import static org.assertj.core.api.Assertions.assertThat;

Expand Down Expand Up @@ -264,6 +258,40 @@ public void readAllContainers() throws Exception{
assertThat(feedResponseIterator1.iterator().hasNext()).isTrue();
}

@Test(groups = { "emulator" }, timeOut = TIMEOUT)
Comment thread
SrinikhilReddy marked this conversation as resolved.
public void crudMultiHashContainerTest() throws Exception{
Comment thread
SrinikhilReddy marked this conversation as resolved.
Outdated
String collectionName = UUID.randomUUID().toString();

PartitionKeyDefinition partitionKeyDefinition = new PartitionKeyDefinition();
partitionKeyDefinition.setKind(PartitionKind.MULTI_HASH);
partitionKeyDefinition.setVersion(PartitionKeyDefinitionVersion.V2);
ArrayList<String> paths = new ArrayList<>();
paths.add("/city");
paths.add("/zipcode");
partitionKeyDefinition.setPaths(paths);

CosmosContainerProperties containerProperties = getCollectionDefinition(collectionName, partitionKeyDefinition);
Comment thread
SrinikhilReddy marked this conversation as resolved.

//MultiHash collection create
CosmosContainerResponse containerResponse = createdDatabase.createContainer(containerProperties);
validateContainerResponse(containerProperties, containerResponse);
assertThat(containerResponse.getProperties().getPartitionKeyDefinition().getKind() == PartitionKind.MULTI_HASH);
assertThat(containerResponse.getProperties().getPartitionKeyDefinition().getPaths().size() == paths.size());
assertThat(containerResponse.getProperties().getPartitionKeyDefinition().getPaths().get(0) == paths.get(0));
Comment thread
SrinikhilReddy marked this conversation as resolved.
assertThat(containerResponse.getProperties().getPartitionKeyDefinition().getPaths().get(1) == paths.get(1));

//MultiHash collection read
CosmosContainer multiHashContainer = createdDatabase.getContainer(collectionName);
containerResponse = multiHashContainer.read();
validateContainerResponse(containerProperties, containerResponse);
assertThat(containerResponse.getProperties().getPartitionKeyDefinition().getKind() == PartitionKind.MULTI_HASH);
assertThat(containerResponse.getProperties().getPartitionKeyDefinition().getPaths().size() == paths.size());
assertThat(containerResponse.getProperties().getPartitionKeyDefinition().getPaths().get(0) == paths.get(0));
assertThat(containerResponse.getProperties().getPartitionKeyDefinition().getPaths().get(1) == paths.get(1));

//MultiHash collection delete
CosmosContainerResponse deleteResponse = multiHashContainer.delete();
}

@Test(groups = { "emulator" }, timeOut = TIMEOUT)
public void queryContainer() throws Exception{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/*
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*
*/

package com.azure.cosmos;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please run the full CI run against the live endpoints.


import com.azure.cosmos.implementation.HttpConstants;
import com.azure.cosmos.implementation.InternalObjectNode;
import com.azure.cosmos.implementation.apachecommons.lang.StringUtils;
import com.azure.cosmos.implementation.routing.PartitionKeyInternal;
import com.azure.cosmos.models.*;
Comment thread
SrinikhilReddy marked this conversation as resolved.
Outdated
import com.azure.cosmos.rx.TestSuiteBase;
import com.azure.cosmos.util.CosmosPagedIterable;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Factory;
import org.testng.annotations.Test;

import java.util.ArrayList;
import java.util.List;
import java.util.UUID;

import static org.assertj.core.api.Assertions.assertThat;

public class CosmosMultiHashTest extends TestSuiteBase {
Comment thread
SrinikhilReddy marked this conversation as resolved.

private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
private String preExistingDatabaseId = CosmosDatabaseForTest.generateId();
private CosmosClient client;
private CosmosDatabase createdDatabase;
private CosmosContainer createdMultiHashContainer;

@Factory(dataProvider = "clientBuilders")
public CosmosMultiHashTest(CosmosClientBuilder clientBuilder) {
Comment thread
SrinikhilReddy marked this conversation as resolved.
super(clientBuilder);
}

@BeforeClass(groups = {"simple"}, timeOut = SETUP_TIMEOUT)
public void before_CosmosItemTest() {

client = getClientBuilder().buildClient();
createdDatabase = createSyncDatabase(client, preExistingDatabaseId);
String collectionName = UUID.randomUUID().toString();

PartitionKeyDefinition partitionKeyDefinition = new PartitionKeyDefinition();
partitionKeyDefinition.setKind(PartitionKind.MULTI_HASH);
partitionKeyDefinition.setVersion(PartitionKeyDefinitionVersion.V2);
ArrayList<String> paths = new ArrayList<>();
paths.add("/city");
paths.add("/zipcode");
partitionKeyDefinition.setPaths(paths);

CosmosContainerProperties containerProperties = getCollectionDefinition(collectionName, partitionKeyDefinition);
Comment thread
SrinikhilReddy marked this conversation as resolved.

//MultiHash collection create
CosmosContainerResponse containerResponse = createdDatabase.createContainer(containerProperties);

//MultiHash collection read
createdMultiHashContainer = createdDatabase.getContainer(collectionName);
}

@AfterClass(groups = {"simple"}, timeOut = SHUTDOWN_TIMEOUT, alwaysRun = true)
public void afterClass() {
logger.info("starting ....");
//MultiHash Collection delete
createdMultiHashContainer.delete();
Comment thread
SrinikhilReddy marked this conversation as resolved.
Outdated
safeDeleteSyncDatabase(createdDatabase);
safeCloseSyncClient(client);
}

@Test(groups = { "simple" }, timeOut = TIMEOUT)
public void itemCRUDTest() throws Exception {
Comment thread
SrinikhilReddy marked this conversation as resolved.
Outdated

List<String> pkIds = new ArrayList<>();
pkIds.add("Redmond");
pkIds.add("98052");

Object[] pkValue = new Object[2];
pkValue[0] = pkIds.get(0);
pkValue[1] = pkIds.get(1);

PartitionKey partitionKey = new PartitionKey(pkValue);

String documentId = UUID.randomUUID().toString();
ObjectNode properties = getDocumentDefinition(documentId, pkIds);
createdMultiHashContainer.createItem(properties);

CosmosItemResponse<ObjectNode> readResponse1 = createdMultiHashContainer.readItem(
documentId, partitionKey, ObjectNode.class);
validateIdOfItemResponse(documentId, readResponse1);
assertThat(readResponse1.getItem().equals(properties));
}

private ObjectNode getDocumentDefinition(String documentId, List<String> pkIds) throws JsonProcessingException {
Comment thread
SrinikhilReddy marked this conversation as resolved.
Outdated

String json = String.format("{ "
+ "\"id\": \"%s\", "
+ "\"city\": \"%s\", "
+ "\"zipcode\": \"%s\", "
+ "\"sgmts\": [[6519456, 1471916863], [2498434, 1455671440]]"
+ "}"
, documentId, pkIds.get(0), pkIds.get(1));
return
OBJECT_MAPPER.readValue(json, ObjectNode.class);
}

private void validateItemResponse(InternalObjectNode containerProperties,
CosmosItemResponse<InternalObjectNode> createResponse) {
// Basic validation
assertThat(BridgeInternal.getProperties(createResponse).getId()).isNotNull();
assertThat(BridgeInternal.getProperties(createResponse).getId())
.as("check Resource Id")
.isEqualTo(containerProperties.getId());
}

private void validateIdOfItemResponse(String expectedId, CosmosItemResponse<ObjectNode> createResponse) {
// Basic validation
assertThat(BridgeInternal.getProperties(createResponse).getId()).isNotNull();
assertThat(BridgeInternal.getProperties(createResponse).getId())
.as("check Resource Id")
.isEqualTo(expectedId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,10 @@ static protected CosmosContainerProperties getCollectionDefinition(String collec
return collectionDefinition;
}

static protected CosmosContainerProperties getCollectionDefinition(String collectionId, PartitionKeyDefinition partitionKeyDefinition) {
return new CosmosContainerProperties(collectionId, partitionKeyDefinition);
}

static protected CosmosContainerProperties getCollectionDefinitionWithRangeRangeIndexWithIdAsPartitionKey() {
return getCollectionDefinitionWithRangeRangeIndex(Collections.singletonList("/id"));
}
Expand Down