-
Notifications
You must be signed in to change notification settings - Fork 30
Add transaction methods extraction class [ECR-3923] #1274
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
Changes from all commits
676f81e
a4ded9d
db470fb
8ca1f17
9552f5b
33dd226
1868419
d47abff
64b753d
c14a62d
b1977f8
67c794f
76bf972
a5cbb3a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
/* | ||
* Copyright 2019 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 | ||
* | ||
* http://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.runtime; | ||
|
||
import static com.google.common.base.Preconditions.checkArgument; | ||
|
||
import com.exonum.binding.core.service.Service; | ||
import com.exonum.binding.core.transaction.TransactionContext; | ||
import com.exonum.binding.core.transaction.TransactionExecutionException; | ||
import java.lang.invoke.MethodHandle; | ||
import java.util.Map; | ||
|
||
/** | ||
* Stores ids of transaction methods and their method handles of a corresponding service. | ||
*/ | ||
final class TransactionInvoker { | ||
private final Service service; | ||
private final Map<Integer, MethodHandle> transactionMethods; | ||
|
||
TransactionInvoker(Service service) { | ||
this.service = service; | ||
this.transactionMethods = | ||
TransactionMethodExtractor.extractTransactionMethods(service.getClass()); | ||
} | ||
|
||
/** | ||
* Invoke the transaction method with a given transaction identifier. | ||
* | ||
* @param transactionId a transaction method identifier | ||
* @param context a transaction execution context | ||
* @param arguments the serialized transaction arguments | ||
* | ||
* @throws IllegalArgumentException if there is no transaction method with given id in a | ||
* corresponding service | ||
* @throws TransactionExecutionException if {@link TransactionExecutionException} was thrown by | ||
* the transaction method, it is propagated | ||
* @throws RuntimeException any other error is wrapped into a {@link RuntimeException} | ||
*/ | ||
void invokeTransaction(int transactionId, byte[] arguments, TransactionContext context) | ||
throws TransactionExecutionException { | ||
checkArgument(transactionMethods.containsKey(transactionId), | ||
"No method with transaction id (%s)", transactionId); | ||
try { | ||
MethodHandle methodHandle = transactionMethods.get(transactionId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fine for this PR, but I'd remind that MHs won't work for any message (next task):
|
||
methodHandle.invoke(service, arguments, context); | ||
} catch (Throwable throwable) { | ||
if (throwable instanceof TransactionExecutionException) { | ||
throw (TransactionExecutionException) throwable; | ||
} else { | ||
throw new RuntimeException(throwable); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Possibly for future improvement) The MH#invoke spec says
So why don't we forbid checked exceptions in transaction methods (except TransactionExecutionException) Please create an issue for that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created ECR-3988 |
||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,111 @@ | ||||||||||||
/* | ||||||||||||
* Copyright 2019 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 | ||||||||||||
* | ||||||||||||
* http://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.runtime; | ||||||||||||
|
||||||||||||
import static com.google.common.base.Preconditions.checkArgument; | ||||||||||||
import static java.util.stream.Collectors.toMap; | ||||||||||||
|
||||||||||||
import com.exonum.binding.core.transaction.TransactionContext; | ||||||||||||
import com.exonum.binding.core.transaction.TransactionMethod; | ||||||||||||
import com.google.common.annotations.VisibleForTesting; | ||||||||||||
|
||||||||||||
import java.lang.invoke.MethodHandle; | ||||||||||||
import java.lang.invoke.MethodHandles; | ||||||||||||
import java.lang.invoke.MethodHandles.Lookup; | ||||||||||||
import java.lang.reflect.Method; | ||||||||||||
import java.util.HashMap; | ||||||||||||
import java.util.Map; | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Finds and validates transaction methods in a service. | ||||||||||||
*/ | ||||||||||||
final class TransactionMethodExtractor { | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Returns a map of transaction ids to transaction methods found in a service class. | ||||||||||||
* | ||||||||||||
* @see TransactionMethod | ||||||||||||
*/ | ||||||||||||
static Map<Integer, MethodHandle> extractTransactionMethods(Class<?> serviceClass) { | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the point of producing method handles of unknown signature? Is it I think some object that can be invoked with context and byte[] and invoke the corresponding transaction method is needed. |
||||||||||||
Map<Integer, Method> transactionMethods = findTransactionMethods(serviceClass); | ||||||||||||
Lookup lookup = MethodHandles.publicLookup() | ||||||||||||
.in(serviceClass); | ||||||||||||
return transactionMethods.entrySet().stream() | ||||||||||||
.peek(tx -> validateTransactionMethod(tx.getValue(), serviceClass)) | ||||||||||||
.collect(toMap(Map.Entry::getKey, | ||||||||||||
(e) -> toMethodHandle(e.getValue(), lookup))); | ||||||||||||
} | ||||||||||||
|
||||||||||||
@VisibleForTesting | ||||||||||||
static Map<Integer, Method> findTransactionMethods(Class<?> serviceClass) { | ||||||||||||
Map<Integer, Method> transactionMethods = new HashMap<>(); | ||||||||||||
while (serviceClass != Object.class) { | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method does several things making testing more complicated:
At least the first step, contributing to this outer loop, must be extracted and tested |
||||||||||||
Method[] classMethods = serviceClass.getDeclaredMethods(); | ||||||||||||
for (Method method : classMethods) { | ||||||||||||
if (method.isAnnotationPresent(TransactionMethod.class)) { | ||||||||||||
TransactionMethod annotation = method.getAnnotation(TransactionMethod.class); | ||||||||||||
int transactionId = annotation.value(); | ||||||||||||
checkDuplicates(transactionMethods, transactionId, serviceClass, method); | ||||||||||||
transactionMethods.put(transactionId, method); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
serviceClass = serviceClass.getSuperclass(); | ||||||||||||
} | ||||||||||||
return transactionMethods; | ||||||||||||
} | ||||||||||||
|
||||||||||||
private static void checkDuplicates(Map<Integer, Method> transactionMethods, int transactionId, | ||||||||||||
Class<?> serviceClass, Method method) { | ||||||||||||
if (transactionMethods.containsKey(transactionId)) { | ||||||||||||
String firstMethodName = transactionMethods.get(transactionId).getName(); | ||||||||||||
String errorMessage = String.format("Service %s has more than one transaction with the same" | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Nit): this one is actually first (given the order of traversal). Also, variable.
Suggested change
|
||||||||||||
+ " id (%s): first: %s; second: %s", | ||||||||||||
serviceClass.getName(), transactionId, firstMethodName, method.getName()); | ||||||||||||
throw new IllegalArgumentException(errorMessage); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Checks that the given transaction method signature is correct. | ||||||||||||
*/ | ||||||||||||
private static void validateTransactionMethod(Method transaction, Class<?> serviceClass) { | ||||||||||||
String errorMessage = String.format("Method %s in a service class %s annotated with" | ||||||||||||
+ " @TransactionMethod should have precisely two parameters of the following types:" | ||||||||||||
+ " 'byte[]' and 'com.exonum.binding.core.transaction.TransactionContext'", | ||||||||||||
transaction.getName(), serviceClass.getName()); | ||||||||||||
checkArgument(transaction.getParameterCount() == 2, errorMessage); | ||||||||||||
Class<?> firstParameter = transaction.getParameterTypes()[0]; | ||||||||||||
Class<?> secondParameter = transaction.getParameterTypes()[1]; | ||||||||||||
checkArgument(firstParameter == byte[].class, | ||||||||||||
String.format(errorMessage | ||||||||||||
+ ". But first parameter type was: %s", firstParameter.getName())); | ||||||||||||
checkArgument(TransactionContext.class.isAssignableFrom(secondParameter), | ||||||||||||
String.format(errorMessage | ||||||||||||
+ ". But second parameter type was: %s", secondParameter.getName())); | ||||||||||||
} | ||||||||||||
|
||||||||||||
private static MethodHandle toMethodHandle(Method method, Lookup lookup) { | ||||||||||||
try { | ||||||||||||
return lookup.unreflect(method); | ||||||||||||
} catch (IllegalAccessException e) { | ||||||||||||
throw new IllegalArgumentException( | ||||||||||||
String.format("Couldn't access method %s", method.getName()), e); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
private TransactionMethodExtractor() {} | ||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
/* | ||
* Copyright 2019 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 | ||
* | ||
* http://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.transaction; | ||
|
||
import com.exonum.binding.common.message.TransactionMessage; | ||
import com.exonum.core.messages.Runtime.ErrorKind; | ||
import com.exonum.core.messages.Runtime.ExecutionError; | ||
import java.lang.annotation.ElementType; | ||
import java.lang.annotation.Retention; | ||
import java.lang.annotation.RetentionPolicy; | ||
import java.lang.annotation.Target; | ||
|
||
/** | ||
* Indicates that a method is a transaction method. The annotated method should execute the | ||
* transaction, possibly modifying the blockchain state. The method should: | ||
* <ul> | ||
* <li>be public | ||
* <li>have exactly two parameters - the | ||
* {@linkplain TransactionMessage#getPayload() serialized transaction arguments} of type | ||
* 'byte[]' and a transaction execution context, which allows to access the information about | ||
* this transaction and modify the blockchain state through the included database fork of | ||
* type '{@link TransactionContext}' in this particular order | ||
* </ul> | ||
* | ||
* <p>The annotated method might throw {@linkplain TransactionExecutionException} if the | ||
* transaction cannot be executed normally and has to be rolled back. The transaction will be | ||
* committed as failed (error kind {@linkplain ErrorKind#SERVICE SERVICE}), the | ||
* {@linkplain ExecutionError#getCode() error code} with the optional description will be saved | ||
* into the storage. The client can request the error code to know the reason of the failure. | ||
* | ||
* <p>The annotated method might also throw {@linkplain RuntimeException} if an unexpected error | ||
* occurs. A correct transaction implementation must not throw such exceptions. The transaction | ||
* will be committed as failed (status "panic"). | ||
* | ||
* @see <a href="https://exonum.com/doc/version/0.13-rc.2/architecture/transactions">Exonum Transactions</a> | ||
* @see <a href="https://exonum.com/doc/version/0.13-rc.2/architecture/services">Exonum Services</a> | ||
*/ | ||
@Target(ElementType.METHOD) | ||
@Retention(RetentionPolicy.RUNTIME) | ||
// TODO: rename to Transaction after migration | ||
public @interface TransactionMethod { | ||
|
||
/** | ||
* Returns the transaction type identifier which is unique within the service. | ||
*/ | ||
int value(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
/* | ||
* Copyright 2019 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 | ||
* | ||
* http://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.runtime; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.junit.jupiter.api.Assertions.assertThrows; | ||
import static org.mockito.Mockito.spy; | ||
import static org.mockito.Mockito.verify; | ||
|
||
import com.exonum.binding.common.hash.HashCode; | ||
import com.exonum.binding.core.service.Node; | ||
import com.exonum.binding.core.service.Service; | ||
import com.exonum.binding.core.storage.database.Snapshot; | ||
import com.exonum.binding.core.transaction.TransactionContext; | ||
import com.exonum.binding.core.transaction.TransactionExecutionException; | ||
import com.exonum.binding.core.transaction.TransactionMethod; | ||
import io.vertx.ext.web.Router; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import org.junit.jupiter.api.Test; | ||
import org.mockito.Mock; | ||
|
||
class TransactionInvokerTest { | ||
|
||
private static final byte[] ARGUMENTS = new byte[0]; | ||
@Mock | ||
private TransactionContext context; | ||
|
||
@Test | ||
void invokeValidServiceTransaction() throws Exception { | ||
ValidService service = spy(new ValidService()); | ||
TransactionInvoker invoker = new TransactionInvoker(service); | ||
invoker.invokeTransaction(ValidService.TRANSACTION_ID, ARGUMENTS, context); | ||
invoker.invokeTransaction(ValidService.TRANSACTION_ID_2, ARGUMENTS, context); | ||
|
||
verify(service).transactionMethod(ARGUMENTS, context); | ||
verify(service).transactionMethod2(ARGUMENTS, context); | ||
} | ||
|
||
@Test | ||
void invokeInvalidTransactionId() { | ||
TransactionInvoker invoker = new TransactionInvoker(new ValidService()); | ||
int invalidTransactionId = Integer.MAX_VALUE; | ||
IllegalArgumentException e = assertThrows(IllegalArgumentException.class, | ||
() -> invoker.invokeTransaction(invalidTransactionId, ARGUMENTS, context)); | ||
assertThat(e.getMessage()) | ||
.contains(String.format("No method with transaction id (%s)", invalidTransactionId)); | ||
} | ||
|
||
@Test | ||
void invokeThrowingTransactionExecutionException() { | ||
TransactionInvoker invoker = new TransactionInvoker(new ThrowingService()); | ||
TransactionExecutionException e = assertThrows(TransactionExecutionException.class, | ||
() -> invoker.invokeTransaction(ThrowingService.TRANSACTION_ID, ARGUMENTS, context)); | ||
assertThat(e.getErrorCode()).isEqualTo(ThrowingService.ERROR_CODE); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please test handling of 'other' service originated exceptions, since it is important to handle them properly. May add a second method to 'ThrowingService'. |
||
@Test | ||
void invokeThrowingServiceException() { | ||
TransactionInvoker invoker = new TransactionInvoker(new ThrowingService()); | ||
RuntimeException e = assertThrows(RuntimeException.class, | ||
() -> invoker.invokeTransaction(ThrowingService.TRANSACTION_ID_2, ARGUMENTS, context)); | ||
assertThat(e.getCause().getClass()).isEqualTo(IllegalArgumentException.class); | ||
} | ||
|
||
static class BasicService implements Service { | ||
|
||
static final int TRANSACTION_ID = 1; | ||
static final int TRANSACTION_ID_2 = 2; | ||
|
||
@Override | ||
public List<HashCode> getStateHashes(Snapshot snapshot) { | ||
return Collections.emptyList(); | ||
} | ||
|
||
@Override | ||
public void createPublicApiHandlers(Node node, Router router) { | ||
// no-op | ||
} | ||
} | ||
|
||
public static class ValidService extends BasicService { | ||
|
||
@TransactionMethod(TRANSACTION_ID) | ||
@SuppressWarnings("WeakerAccess") // Should be accessible | ||
public void transactionMethod(byte[] arguments, TransactionContext context) { | ||
} | ||
|
||
@TransactionMethod(TRANSACTION_ID_2) | ||
@SuppressWarnings("WeakerAccess") // Should be accessible | ||
public void transactionMethod2(byte[] arguments, TransactionContext context) { | ||
} | ||
} | ||
|
||
public static class ThrowingService extends BasicService { | ||
|
||
static final byte ERROR_CODE = 18; | ||
static final String ERROR_MESSAGE = "Service originated exception"; | ||
|
||
@TransactionMethod(TRANSACTION_ID) | ||
public void transactionMethod(byte[] arguments, TransactionContext context) | ||
throws TransactionExecutionException { | ||
throw new TransactionExecutionException(ERROR_CODE); | ||
} | ||
|
||
@TransactionMethod(TRANSACTION_ID_2) | ||
public void transactionMethod2(byte[] arguments, TransactionContext context) | ||
throws TransactionExecutionException { | ||
throw new IllegalArgumentException(ERROR_MESSAGE); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Meta) We must extend the task on errors to cover routing errors — this is an instance of such error.