Unify the context in transaction-like methods: [ECR-4303] #1462
Conversation
exonum-java-binding/CHANGELOG.md
Outdated
| <!-- TODO: Shall we rename it into `ExecutionContext` (as | ||
| TransactionExecutionException has become ExecutionException)? | ||
| --> |
There was a problem hiding this comment.
I think renaming the type to ExecutionContext makes sense.
There was a problem hiding this comment.
I like renaming to ExecutionContext. For example, TransactionContext at initialize leads to a question "what kind of transaction do I have here?".
There was a problem hiding this comment.
OK, thanks, will do here.
Use `TransactionContext` in transaction-like methods: - Service#initialize, #resume, #before- and #afterTransactions - Configurable That enables easier re-use of @transaction methods, as they already accept TransactionContext; and makes the API a little easier to learn because there are fewer immediate abstractions (though you will have to use TransactionContext#getBlockchainData to access the data anyway).
dmitry-timofeev
force-pushed
the
unify-contexts-ECR-4303
branch
from
812641d to
7d39fd1
Compare
exonum-java-binding/CHANGELOG.md
Outdated
| <!-- TODO: Shall we rename it into `ExecutionContext` (as | ||
| TransactionExecutionException has become ExecutionException)? | ||
| --> |
There was a problem hiding this comment.
I think renaming the type to ExecutionContext makes sense.
| * Returns SHA-256 hash of the {@linkplain TransactionMessage transaction message} that | ||
| * carried the payload of the transaction. | ||
| * Each transaction message is uniquely identified by its hash; the messages are persisted | ||
| * carried the payload of the transaction; or a zero hash if no message corresponds to this |
There was a problem hiding this comment.
Question as a non-Java person: would it make sense to return Optional.empty() rather than a zero hash? IMO, this would be more semantically sound.
There was a problem hiding this comment.
Yes, it would make sense, since we are on Java 11, it has acquired some methods that make it easier to use than previously, where it added some friction.
| private String serviceName; | ||
| private Integer serviceId; | ||
|
|
||
| // todo: Init hash and author to zeroes? Or always leave to the client (as now)? |
There was a problem hiding this comment.
Now is ok because the context is created on the framework site.
Redefine the TransactionContext: - Rename it to ExecutionContext and move to service package. - Return Optional for optional context properties. Use `ExecutionContext` in transaction-like methods: - Service#initialize, #resume, #before- and #afterTransactions - Configurable That enables easier re-use of @transaction methods, as they already accept ExecutionContext; and makes the API a little easier to learn because there are fewer immediate abstractions (though you will have to use ExecutionContext#getBlockchainData to access the data anyway). Also, move ex-transaction classes to service package.
Overview
Use
TransactionContextin transaction-like methods:That enables easier re-use of @transaction methods, as they
already accept TransactionContext; and makes the API a little
easier to learn because there are fewer immediate abstractions
(though you will have to use TransactionContext#getBlockchainData
to access the data anyway).
See:
Definition of Done