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.

Sign up for GitHub

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

Fixed diagnostics information and other APIs on cosmos stored procedure response #16946

Merged
merged 3 commits into from
Oct 29, 2020
Merged

Fixed diagnostics information and other APIs on cosmos stored procedure response #16946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4ec4f45
Fixed diagnostics information and other APIs on cosmos stored procedu…
kushagraThapar Oct 29, 2020
02b1e97
Code review comments
kushagraThapar Oct 29, 2020
050a8b9
Fixing conflict resolution policy CRUD test
kushagraThapar Oct 29, 2020
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
4 changes: 4 additions & 0 deletions ...s/azure-cosmos/src/main/java/com/azure/cosmos/implementation/StoredProcedureResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,4 +135,8 @@ public String getScriptLog() {
public CosmosDiagnostics getCosmosDiagnostics() {
return this.response.getCosmosDiagnostics();
}

public RxDocumentServiceResponse getRxDocumentServiceResponse() {
return response;
}
}
5 changes: 0 additions & 5 deletions sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/models/CosmosResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@ public class CosmosResponse<T> {
this.properties = properties;
}

// Only used in CosmosStoredProcedureResponse compatibility with StoredProcedureResponse
CosmosResponse(StoredProcedureResponse response) {
this.resourceResponseWrapper = null;
}

/**
* Gets properties.
*
Expand Down
13 changes: 9 additions & 4 deletions ...mos/azure-cosmos/src/main/java/com/azure/cosmos/models/CosmosStoredProcedureResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ public class CosmosStoredProcedureResponse extends CosmosResponse<CosmosStoredPr
}

CosmosStoredProcedureResponse(StoredProcedureResponse response) {
super(response);
kushagraThapar marked this conversation as resolved.
Show resolved Hide resolved
super(new ResourceResponse<>(response.getRxDocumentServiceResponse(), StoredProcedure.class));
this.storedProcedureResponse = response;

}

/**
Expand Down Expand Up @@ -96,7 +95,10 @@ public double getRequestCharge() {
* @return the response as a string.
*/
public String getResponseAsString() {
return this.storedProcedureResponse.getResponseAsString();
if (storedProcedureResponse != null) {
return storedProcedureResponse.getResponseAsString();
}
return null;
}

/**
Expand All @@ -105,6 +107,9 @@ public String getResponseAsString() {
* @return the output string from the stored procedure console.log() statements.
*/
public String getScriptLog() {
return this.storedProcedureResponse.getScriptLog();
if (storedProcedureResponse != null) {
return this.storedProcedureResponse.getScriptLog();
}
return null;
}
}
53 changes: 43 additions & 10 deletions sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/CosmosSyncStoredProcTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
package com.azure.cosmos;

import com.azure.cosmos.implementation.HttpConstants;
import com.azure.cosmos.models.CosmosStoredProcedureResponse;
import com.azure.cosmos.models.CosmosQueryRequestOptions;
import com.azure.cosmos.models.CosmosStoredProcedureProperties;
import com.azure.cosmos.models.CosmosStoredProcedureRequestOptions;
import com.azure.cosmos.models.CosmosQueryRequestOptions;
import com.azure.cosmos.models.CosmosStoredProcedureResponse;
import com.azure.cosmos.models.PartitionKey;
import com.azure.cosmos.models.SqlQuerySpec;
import com.azure.cosmos.rx.TestSuiteBase;
Expand All @@ -17,10 +17,10 @@
import org.testng.annotations.Factory;
import org.testng.annotations.Test;

import java.net.URLDecoder;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.UUID;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -58,13 +58,15 @@ public void createStoredProcedure() throws Exception {

CosmosStoredProcedureResponse response = container.getScripts().createStoredProcedure(storedProcedureDef);
validateResponse(storedProcedureDef, response);
validateDiagnostics(response, false);

storedProcedureDef.setId(UUID.randomUUID().toString());
storedProcedureDef.setBody("function() {var x = 11;}");
CosmosStoredProcedureResponse response1 = container.getScripts()
.createStoredProcedure(storedProcedureDef,
new CosmosStoredProcedureRequestOptions());
validateResponse(storedProcedureDef, response1);
validateDiagnostics(response1, false);

}

Expand All @@ -74,6 +76,7 @@ public void createSproc_alreadyExists() throws Exception {

CosmosStoredProcedureResponse response = container.getScripts().createStoredProcedure(storedProcedureDef);
validateResponse(storedProcedureDef, response);
validateDiagnostics(response, false);

// Test for conflict
try {
Expand All @@ -90,14 +93,17 @@ public void readStoredProcedure() throws Exception {

CosmosStoredProcedureResponse response = container.getScripts().createStoredProcedure(storedProcedureDef);
validateResponse(storedProcedureDef, response);
validateDiagnostics(response, false);

CosmosStoredProcedure storedProcedure = container.getScripts().getStoredProcedure(storedProcedureDef.getId());
CosmosStoredProcedureResponse readResponse = storedProcedure.read();
validateResponse(storedProcedureDef, readResponse);
validateDiagnostics(readResponse, false);

CosmosStoredProcedureResponse readResponse2 =
storedProcedure.read(new CosmosStoredProcedureRequestOptions());
validateResponse(storedProcedureDef, readResponse2);
validateDiagnostics(readResponse2, false);
}

@Test(groups = {"simple"}, timeOut = TIMEOUT)
Expand All @@ -106,25 +112,29 @@ public void replaceStoredProcedure() throws Exception {

CosmosStoredProcedureResponse response = container.getScripts().createStoredProcedure(storedProcedureDef);
validateResponse(storedProcedureDef, response);
validateDiagnostics(response, false);

CosmosStoredProcedureResponse readResponse = container.getScripts()
.getStoredProcedure(storedProcedureDef.getId())
.read();
validateResponse(storedProcedureDef, readResponse);
validateDiagnostics(readResponse, false);
//replace
storedProcedureDef = readResponse.getProperties();
storedProcedureDef.setBody("function(){ var y = 20;}");
CosmosStoredProcedureResponse replaceResponse = container.getScripts()
.getStoredProcedure(storedProcedureDef.getId())
.replace(storedProcedureDef);
validateResponse(storedProcedureDef, replaceResponse);
validateDiagnostics(replaceResponse, false);

storedProcedureDef.setBody("function(){ var z = 2;}");
CosmosStoredProcedureResponse replaceResponse2 = container.getScripts()
.getStoredProcedure(storedProcedureDef.getId())
.replace(storedProcedureDef,
new CosmosStoredProcedureRequestOptions());
validateResponse(storedProcedureDef, replaceResponse2);
validateDiagnostics(replaceResponse2, false);

}

Expand All @@ -143,6 +153,8 @@ public void deleteStoredProcedure() throws Exception {

CosmosStoredProcedureResponse response = container.getScripts().createStoredProcedure(storedProcedureDef);
validateResponse(storedProcedureDef, response);
validateDiagnostics(response, false);

container.getScripts()
.getStoredProcedure(storedProcedureDef.getId())
.delete();
Expand All @@ -164,7 +176,8 @@ public void executeStoredProcedure() throws Exception {
" }" +
"}");

container.getScripts().createStoredProcedure(storedProcedure);
CosmosStoredProcedureResponse response = container.getScripts().createStoredProcedure(storedProcedure);
validateDiagnostics(response, false);
CosmosStoredProcedureRequestOptions options = new CosmosStoredProcedureRequestOptions();
options.setPartitionKey(PartitionKey.NONE);
CosmosStoredProcedureResponse executeResponse = container.getScripts()
Expand All @@ -173,6 +186,7 @@ public void executeStoredProcedure() throws Exception {

assertThat(executeResponse.getActivityId()).isNotEmpty();
assertThat(executeResponse.getScriptLog()).isNull();
validateDiagnostics(executeResponse, true);
}

@Test(groups = "simple", timeOut = TIMEOUT)
Expand All @@ -192,7 +206,8 @@ public void executeStoredProcedureWithScriptLoggingEnabled() throws Exception {
" }" +
"}");

container.getScripts().createStoredProcedure(storedProcedure);
CosmosStoredProcedureResponse response = container.getScripts().createStoredProcedure(storedProcedure);
validateDiagnostics(response, false);
CosmosStoredProcedureRequestOptions options = new CosmosStoredProcedureRequestOptions();
options.setScriptLoggingEnabled(true);
options.setPartitionKey(PartitionKey.NONE);
Expand All @@ -203,12 +218,15 @@ public void executeStoredProcedureWithScriptLoggingEnabled() throws Exception {

String logResult = "The value of x is 1.";
assertThat(executeResponse.getScriptLog()).isEqualTo(logResult);
validateDiagnostics(executeResponse, true);
}

@Test(groups = {"simple"}, timeOut = TIMEOUT)
private void readAllSprocs() throws Exception {
public void readAllSprocs() throws Exception {
CosmosStoredProcedureProperties storedProcedureDef = getCosmosStoredProcedureProperties();
container.getScripts().createStoredProcedure(storedProcedureDef);
CosmosStoredProcedureResponse response =
container.getScripts().createStoredProcedure(storedProcedureDef);
validateDiagnostics(response, false);

CosmosQueryRequestOptions cosmosQueryRequestOptions = new CosmosQueryRequestOptions();

Expand All @@ -219,9 +237,10 @@ private void readAllSprocs() throws Exception {
}

@Test(groups = {"simple"}, timeOut = TIMEOUT)
private void querySprocs() throws Exception {
public void querySprocs() throws Exception {
CosmosStoredProcedureProperties properties = getCosmosStoredProcedureProperties();
container.getScripts().createStoredProcedure(properties);
CosmosStoredProcedureResponse response = container.getScripts().createStoredProcedure(properties);
validateDiagnostics(response, false);

String query = String.format("SELECT * from c where c.id = '%s'", properties.getId());
CosmosQueryRequestOptions cosmosQueryRequestOptions = new CosmosQueryRequestOptions();
Expand All @@ -245,4 +264,18 @@ private void validateResponse(CosmosStoredProcedureProperties properties,
.isEqualTo(properties.getId());

}

private void validateDiagnostics(CosmosStoredProcedureResponse response, boolean storedProcedureExecuted ) {
CosmosDiagnostics diagnostics = response.getDiagnostics();
Duration duration = response.getDuration();
Map<String, String> responseHeaders = response.getResponseHeaders();
assertThat(diagnostics).isNotNull();
assertThat(duration).isNotNull();
assertThat(duration.toMillis()).isGreaterThan(0);
assertThat(responseHeaders).isNotEmpty();
if (storedProcedureExecuted) {
String responseAsString = response.getResponseAsString();
assertThat(responseAsString).isNotNull();
}
}
}
2 changes: 1 addition & 1 deletion ...mos/azure-cosmos/src/test/java/com/azure/cosmos/rx/MultiMasterConflictResolutionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public void conflictResolutionPolicyCRUD() {
// when (e.StatusCode == HttpStatusCode.BadRequest)
CosmosException dce = Utils.as(e, CosmosException.class);
if (dce != null && dce.getStatusCode() == 400) {
assertThat(dce.getMessage()).contains("Invalid path '\\/a\\/b' for last writer wins conflict resolution");
assertThat(dce.getMessage()).contains("Invalid path '\\\\\\/a\\\\\\/b' for last writer wins conflict resolution");
Copy link
Contributor

Choose a reason for hiding this comment

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

is this because of service side message change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that is the reason :)

} else {
throw e;
}
Expand Down