Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Error response handling for permissions APIs #736

Merged
merged 8 commits into from
Feb 1, 2019
Merged

Error response handling for permissions APIs #736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d5590a0
Generifying account/node result object
lucassaldanha Jan 31, 2019
067567c
Added null/empty check on whitelist ops
lucassaldanha Jan 31, 2019
1ce91e7
Updating successful whitelist operation to return Success instead of …
lucassaldanha Jan 31, 2019
1932117
Updating error messages
lucassaldanha Feb 1, 2019
58186b4
Moving Success string into JsonRpcSuccessful response
lucassaldanha Feb 1, 2019
c40e12b
Merge branch 'master' into nc-2113
lucassaldanha Feb 1, 2019
22dd093
Merge branch 'master' into nc-2113
lucassaldanha Feb 1, 2019
6f20764
Updating unit tests
lucassaldanha Feb 1, 2019
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: 2 additions & 2 deletions ...asys/pantheon/tests/acceptance/dsl/condition/perm/AddAccountsToWhitelistSuccessfully.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public AddAccountsToWhitelistSuccessfully(

@Override
public void verify(final Node node) {
Boolean result = node.execute(transaction);
assertThat(result).isTrue();
String result = node.execute(transaction);
assertThat(result).isEqualTo("Success");
}
}
4 changes: 2 additions & 2 deletions ...c/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/condition/perm/AddNodeSuccess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public AddNodeSuccess(final PermAddNodeTransaction transactions) {

@Override
public void verify(final Node node) {
final Boolean response = node.execute(transaction);
assertThat(response).isTrue();
final String response = node.execute(transaction);
assertThat(response).isEqualTo("Success");
}
}
4 changes: 2 additions & 2 deletions ...pantheon/tests/acceptance/dsl/condition/perm/RemoveAccountsFromWhitelistSuccessfully.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public RemoveAccountsFromWhitelistSuccessfully(

@Override
public void verify(final Node node) {
Boolean result = node.execute(transaction);
assertThat(result).isTrue();
String result = node.execute(transaction);
assertThat(result).isEqualTo("Success");
}
}
4 changes: 2 additions & 2 deletions ...est/java/tech/pegasys/pantheon/tests/acceptance/dsl/condition/perm/RemoveNodeSuccess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public RemoveNodeSuccess(final PermRemoveNodeTransaction transaction) {

@Override
public void verify(final Node node) {
final Boolean response = node.execute(transaction);
assertThat(response).isTrue();
final String response = node.execute(transaction);
assertThat(response).isEqualTo("Success");
}
}
8 changes: 4 additions & 4 deletions ...s/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/transaction/ResponseTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ public static class SignersBlockResponse extends Response<List<Address>> {}

public static class ProposalsResponse extends Response<Map<Address, Boolean>> {}

public static class AddAccountsToWhitelistResponse extends Response<Boolean> {}
public static class AddAccountsToWhitelistResponse extends Response<String> {}

public static class RemoveAccountsFromWhitelistResponse extends Response<Boolean> {}
public static class RemoveAccountsFromWhitelistResponse extends Response<String> {}

public static class GetAccountsWhitelistResponse extends Response<List<String>> {}

public static class AddNodeResponse extends Response<Boolean> {}
public static class AddNodeResponse extends Response<String> {}

public static class RemoveNodeResponse extends Response<Boolean> {}
public static class RemoveNodeResponse extends Response<String> {}

public static class GetNodesWhitelistResponse extends Response<List<String>> {}
}
6 changes: 3 additions & 3 deletions ...pantheon/tests/acceptance/dsl/transaction/perm/PermAddAccountsToWhitelistTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import java.io.IOException;
import java.util.List;

public class PermAddAccountsToWhitelistTransaction implements Transaction<Boolean> {
public class PermAddAccountsToWhitelistTransaction implements Transaction<String> {

private final List<String> accounts;

Expand All @@ -30,10 +30,10 @@ public PermAddAccountsToWhitelistTransaction(final List<String> accounts) {
}

@Override
public Boolean execute(final JsonRequestFactories node) {
public String execute(final JsonRequestFactories node) {
try {
AddAccountsToWhitelistResponse response = node.perm().addAccountsToWhitelist(accounts).send();
assertThat(response.getResult()).isTrue();
assertThat(response.getResult()).isEqualTo("Success");
return response.getResult();
} catch (IOException e) {
throw new RuntimeException(e);
Expand Down
4 changes: 2 additions & 2 deletions ...a/tech/pegasys/pantheon/tests/acceptance/dsl/transaction/perm/PermAddNodeTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@
import java.io.IOException;
import java.util.List;

public class PermAddNodeTransaction implements Transaction<Boolean> {
public class PermAddNodeTransaction implements Transaction<String> {
private final List<String> enodeList;

public PermAddNodeTransaction(final List<String> enodeList) {
this.enodeList = enodeList;
}

@Override
public Boolean execute(final JsonRequestFactories node) {
public String execute(final JsonRequestFactories node) {
try {
final AddNodeResponse result = node.perm().addNodesToWhitelist(enodeList).send();
assertThat(result).isNotNull();
Expand Down
6 changes: 3 additions & 3 deletions ...eon/tests/acceptance/dsl/transaction/perm/PermRemoveAccountsFromWhitelistTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import java.io.IOException;
import java.util.List;

public class PermRemoveAccountsFromWhitelistTransaction implements Transaction<Boolean> {
public class PermRemoveAccountsFromWhitelistTransaction implements Transaction<String> {

private final List<String> accounts;

Expand All @@ -30,11 +30,11 @@ public PermRemoveAccountsFromWhitelistTransaction(final List<String> accounts) {
}

@Override
public Boolean execute(final JsonRequestFactories node) {
public String execute(final JsonRequestFactories node) {
try {
RemoveAccountsFromWhitelistResponse response =
node.perm().removeAccountsFromWhitelist(accounts).send();
assertThat(response.getResult()).isTrue();
assertThat(response.getResult()).isEqualTo("Success");
return response.getResult();
} catch (IOException e) {
throw new RuntimeException(e);
Expand Down
4 changes: 2 additions & 2 deletions ...ech/pegasys/pantheon/tests/acceptance/dsl/transaction/perm/PermRemoveNodeTransaction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@
import java.io.IOException;
import java.util.List;

public class PermRemoveNodeTransaction implements Transaction<Boolean> {
public class PermRemoveNodeTransaction implements Transaction<String> {
private final List<String> enodeList;

public PermRemoveNodeTransaction(final List<String> enodeList) {
this.enodeList = enodeList;
}

@Override
public Boolean execute(final JsonRequestFactories node) {
public String execute(final JsonRequestFactories node) {
try {
final RemoveNodeResponse result = node.perm().removeNodesFromWhitelist(enodeList).send();
assertThat(result).isNotNull();
Expand Down
9 changes: 6 additions & 3 deletions .../pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermAddAccountsToWhitelist.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse;
import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse;
import tech.pegasys.pantheon.ethereum.permissioning.AccountWhitelistController;
import tech.pegasys.pantheon.ethereum.permissioning.AccountWhitelistController.AddResult;
import tech.pegasys.pantheon.ethereum.permissioning.WhitelistOperationResult;

import java.util.List;

Expand All @@ -44,9 +44,12 @@ public String getName() {
@SuppressWarnings("unchecked")
public JsonRpcResponse response(final JsonRpcRequest request) {
final List<String> accountsList = parameters.required(request.getParams(), 0, List.class);
final AddResult addResult = whitelistController.addAccounts(accountsList);
final WhitelistOperationResult addResult = whitelistController.addAccounts(accountsList);

switch (addResult) {
case ERROR_EMPTY_ENTRY:
return new JsonRpcErrorResponse(
request.getId(), JsonRpcError.ACCOUNT_WHITELIST_EMPTY_ENTRY);
case ERROR_INVALID_ENTRY:
return new JsonRpcErrorResponse(
request.getId(), JsonRpcError.ACCOUNT_WHITELIST_INVALID_ENTRY);
Expand All @@ -57,7 +60,7 @@ public JsonRpcResponse response(final JsonRpcRequest request) {
return new JsonRpcErrorResponse(
request.getId(), JsonRpcError.ACCOUNT_WHITELIST_DUPLICATED_ENTRY);
case SUCCESS:
return new JsonRpcSuccessResponse(request.getId(), true);
return new JsonRpcSuccessResponse(request.getId());
default:
throw new IllegalStateException("Unmapped result from AccountWhitelistController");
}
Expand Down
4 changes: 3 additions & 1 deletion ...sys/pantheon/ethereum/jsonrpc/internal/methods/permissioning/PermAddNodesToWhitelist.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ public JsonRpcResponse response(final JsonRpcRequest req) {

switch (nodesWhitelistResult.result()) {
case SUCCESS:
return new JsonRpcSuccessResponse(req.getId(), true);
return new JsonRpcSuccessResponse(req.getId());
case ERROR_EMPTY_ENTRY:
return new JsonRpcErrorResponse(req.getId(), JsonRpcError.NODE_WHITELIST_EMPTY_ENTRY);
case ERROR_EXISTING_ENTRY:
return new JsonRpcErrorResponse(req.getId(), JsonRpcError.NODE_WHITELIST_EXISTING_ENTRY);
case ERROR_DUPLICATED_ENTRY:
Expand Down
9 changes: 6 additions & 3 deletions ...heon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveAccountsFromWhitelist.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse;
import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse;
import tech.pegasys.pantheon.ethereum.permissioning.AccountWhitelistController;
import tech.pegasys.pantheon.ethereum.permissioning.AccountWhitelistController.RemoveResult;
import tech.pegasys.pantheon.ethereum.permissioning.WhitelistOperationResult;

import java.util.List;

Expand All @@ -44,9 +44,12 @@ public String getName() {
@SuppressWarnings("unchecked")
public JsonRpcResponse response(final JsonRpcRequest request) {
final List<String> accountsList = parameters.required(request.getParams(), 0, List.class);
final RemoveResult removeResult = whitelistController.removeAccounts(accountsList);
final WhitelistOperationResult removeResult = whitelistController.removeAccounts(accountsList);

switch (removeResult) {
case ERROR_EMPTY_ENTRY:
return new JsonRpcErrorResponse(
request.getId(), JsonRpcError.ACCOUNT_WHITELIST_EMPTY_ENTRY);
case ERROR_INVALID_ENTRY:
return new JsonRpcErrorResponse(
request.getId(), JsonRpcError.ACCOUNT_WHITELIST_INVALID_ENTRY);
Expand All @@ -57,7 +60,7 @@ public JsonRpcResponse response(final JsonRpcRequest request) {
return new JsonRpcErrorResponse(
request.getId(), JsonRpcError.ACCOUNT_WHITELIST_DUPLICATED_ENTRY);
case SUCCESS:
return new JsonRpcSuccessResponse(request.getId(), true);
return new JsonRpcSuccessResponse(request.getId());
default:
throw new IllegalStateException("Unmapped result from AccountWhitelistController");
}
Expand Down
4 changes: 3 additions & 1 deletion ...antheon/ethereum/jsonrpc/internal/methods/permissioning/PermRemoveNodesFromWhitelist.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ public JsonRpcResponse response(final JsonRpcRequest req) {

switch (nodesWhitelistResult.result()) {
case SUCCESS:
return new JsonRpcSuccessResponse(req.getId(), true);
return new JsonRpcSuccessResponse(req.getId());
case ERROR_EMPTY_ENTRY:
return new JsonRpcErrorResponse(req.getId(), JsonRpcError.NODE_WHITELIST_EMPTY_ENTRY);
case ERROR_ABSENT_ENTRY:
return new JsonRpcErrorResponse(req.getId(), JsonRpcError.NODE_WHITELIST_MISSING_ENTRY);
case ERROR_DUPLICATED_ENTRY:
Expand Down
21 changes: 11 additions & 10 deletions .../src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,21 @@ public enum JsonRpcError {
COINBASE_NOT_SPECIFIED(-32000, "Coinbase must be explicitly specified"),

// Permissioning errors
ACCOUNT_WHITELIST_NOT_SET(-32000, "Account whitelist hasn't been set"),
ACCOUNT_WHITELIST_DUPLICATED_ENTRY(-32000, "Request can't contain duplicated entries"),
ACCOUNT_WHITELIST_EXISTING_ENTRY(-32000, "Can't add existing account to whitelist"),
ACCOUNT_WHITELIST_ABSENT_ENTRY(-32000, "Can't remove absent account from whitelist"),
ACCOUNT_WHITELIST_INVALID_ENTRY(-32000, "Can't add invalid account address to the whitelist"),
ACCOUNT_WHITELIST_NOT_SET(-32000, "Account whitelist has not been set"),
ACCOUNT_WHITELIST_EMPTY_ENTRY(-32000, "Request contains an empty list of accounts"),
ACCOUNT_WHITELIST_INVALID_ENTRY(-32000, "Request contains an invalid account"),
ACCOUNT_WHITELIST_DUPLICATED_ENTRY(-32000, "Request contains duplicate accounts"),
ACCOUNT_WHITELIST_EXISTING_ENTRY(-32000, "Cannot add an existing account to whitelist"),
ACCOUNT_WHITELIST_ABSENT_ENTRY(-32000, "Cannot remove an absent account from whitelist"),

// Node whitelist errors
NODE_WHITELIST_NOT_SET(-32000, "Node whitelist has not been set"),
NODE_WHITELIST_DUPLICATED_ENTRY(-32000, "Request can't contain duplicated node entries"),
NODE_WHITELIST_EXISTING_ENTRY(-32000, "Node whitelist can't contain duplicated node entries"),
NODE_WHITELIST_MISSING_ENTRY(-32000, "Node whitelist does not contain a specified node"),
NODE_WHITELIST_INVALID_ENTRY(-32000, "Unable to add invalid node to the node whitelist"),
NODE_WHITELIST_EMPTY_ENTRY(-32000, "Request contains an empty list of nodes"),
NODE_WHITELIST_INVALID_ENTRY(-32000, "Request contains an invalid node"),
NODE_WHITELIST_DUPLICATED_ENTRY(-32000, "Request contains duplicate nodes"),
NODE_WHITELIST_EXISTING_ENTRY(-32000, "Cannot add an existing node to whitelist"),
NODE_WHITELIST_MISSING_ENTRY(-32000, "Cannot remove an absent node from whitelist"),

// Private transaction errors
ENCLAVE_IS_DOWN(-32000, "Enclave is down");

private final int code;
Expand Down
5 changes: 5 additions & 0 deletions ...java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcSuccessResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ public JsonRpcSuccessResponse(final Object id, final Object result) {
this.result = result;
}

public JsonRpcSuccessResponse(final Object id) {
this.id = id;
this.result = "Success";
}

@JsonGetter("id")
public Object getId() {
return id;
Expand Down
30 changes: 23 additions & 7 deletions ...theon/ethereum/jsonrpc/internal/methods/permissioning/PermAddAccountsToWhitelistTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse;
import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse;
import tech.pegasys.pantheon.ethereum.permissioning.AccountWhitelistController;
import tech.pegasys.pantheon.ethereum.permissioning.AccountWhitelistController.AddResult;
import tech.pegasys.pantheon.ethereum.permissioning.WhitelistOperationResult;

import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -55,10 +55,10 @@ public void getNameShouldReturnExpectedName() {
}

@Test
public void whenAccountsAreAddedToWhitelistShouldReturnTrue() {
public void whenAccountsAreAddedToWhitelistShouldReturnSuccess() {
List<String> accounts = Arrays.asList("0x0", "0x1");
JsonRpcResponse expectedResponse = new JsonRpcSuccessResponse(null, true);
when(accountWhitelist.addAccounts(eq(accounts))).thenReturn(AddResult.SUCCESS);
JsonRpcResponse expectedResponse = new JsonRpcSuccessResponse(null);
when(accountWhitelist.addAccounts(eq(accounts))).thenReturn(WhitelistOperationResult.SUCCESS);

JsonRpcResponse actualResponse = method.response(request(accounts));

Expand All @@ -69,7 +69,8 @@ public void whenAccountsAreAddedToWhitelistShouldReturnTrue() {
public void whenAccountIsInvalidShouldReturnInvalidAccountErrorResponse() {
JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(null, JsonRpcError.ACCOUNT_WHITELIST_INVALID_ENTRY);
when(accountWhitelist.addAccounts(any())).thenReturn(AddResult.ERROR_INVALID_ENTRY);
when(accountWhitelist.addAccounts(any()))
.thenReturn(WhitelistOperationResult.ERROR_INVALID_ENTRY);

JsonRpcResponse actualResponse = method.response(request(new ArrayList<>()));

Expand All @@ -80,7 +81,8 @@ public void whenAccountIsInvalidShouldReturnInvalidAccountErrorResponse() {
public void whenAccountExistsShouldReturnExistingEntryErrorResponse() {
JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(null, JsonRpcError.ACCOUNT_WHITELIST_EXISTING_ENTRY);
when(accountWhitelist.addAccounts(any())).thenReturn(AddResult.ERROR_EXISTING_ENTRY);
when(accountWhitelist.addAccounts(any()))
.thenReturn(WhitelistOperationResult.ERROR_EXISTING_ENTRY);

JsonRpcResponse actualResponse = method.response(request(new ArrayList<>()));

Expand All @@ -91,7 +93,21 @@ public void whenAccountExistsShouldReturnExistingEntryErrorResponse() {
public void whenInputHasDuplicatedAccountsShouldReturnDuplicatedEntryErrorResponse() {
JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(null, JsonRpcError.ACCOUNT_WHITELIST_DUPLICATED_ENTRY);
when(accountWhitelist.addAccounts(any())).thenReturn(AddResult.ERROR_DUPLICATED_ENTRY);
when(accountWhitelist.addAccounts(any()))
.thenReturn(WhitelistOperationResult.ERROR_DUPLICATED_ENTRY);

JsonRpcResponse actualResponse = method.response(request(new ArrayList<>()));

assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse);
}

@Test
public void whenEmptyListOnRequestShouldReturnEmptyEntryErrorResponse() {
JsonRpcResponse expectedResponse =
new JsonRpcErrorResponse(null, JsonRpcError.ACCOUNT_WHITELIST_EMPTY_ENTRY);

when(accountWhitelist.addAccounts(eq(new ArrayList<>())))
.thenReturn(WhitelistOperationResult.ERROR_EMPTY_ENTRY);

JsonRpcResponse actualResponse = method.response(request(new ArrayList<>()));

Expand Down
Loading