This is another larger refactoring, sorry ;-)
But the structure was just not correct before. We had handled multiple
trades with multiple results and that is error prone. Now each class
has much more clear responsibilities. Also the result enums are not
changes so that they are better separated between result based and
service bases states.
All different states are still hard to test. We should set up some mock
service to simulate confirmations counting up and error scenarios.
Change log text.
Those banned nodes are quite old and no need to restart the app. In case
we need to really ban a node (the one we banned are just revoked) we can
use the global notification to alert users to restart.
- Do not use immutability for AutoConfirmSettings as it makes setting
values cumbersome.
- Add btc validator for trade limit
- Make AutoConfirmSettings an optional and add find method by currency
code to be better prepared when used for other coins.
- Add static getDefaultForXmr to AutoConfirmSettings
- Move listener creation to init method
Remove testKey test (tested uid of model)
Refactor:
- Rename uid to id (we do not have a strict guarantee for uniqueness)
- Move id from model to request (its the id of the request)
In cased new fields are added to PB this case is expected as well in
development if protbuf definitions have changed over time and persisted
data do not match the new definition. We don't want this log to be
that verbose.
to add auto confirm for other currencies in the future. The generic part
is only used where we would have issues with backward compatibility like
in the protobuf objects. Most of the current classes are kept XMR
specific and could be generalized once we add other assets, but that
would be an internal refactoring without breaking any network or
storage data. I think it would be premature to go further here as we
don't know the details of other use cases. I added the methods used from
clients to AutoConfirmResult, not sure if the API is well defined by
that, but as said that could become subject of a future refactoring once
another auto confirm feature gets added. Goal of that refactoring was
to avoid that we need more fields for trade and the the UI would have to
deal with lots of switch cases based on currency.
Sorry that is a larger commit, would have been hard to break up...
- Replace httpClient.toString() with httpClient.getBaseUrl() as toString
would deliver too much for those use cases.
- Add @Nullable
- Log improvements
instead which stores the stateName. [1]
- Adjust protobuf methods
- Add UNDEFINED to AutoConfirmResult.State to support cases where we
get no data to set the enum.
- Add NO_MATCH_FOUND (used in follow up commits)
- Refactoring: Improve constructors
[1]
Enums in protobuf are not well supported. They are global so an enum
with name (e.g. State) inside Trade conflicts with another enum inside
Message with the same name. So they do not reflect encapsulation in the
class like in java.
We moved over time to the strategy to use strings (from enum.name())
instead of the enum, avoiding also cumbersome fromProto and toProto
code and being more flexible with updates.
The autoConfirmResultState enum inside Trade was a bit confusing to me
as it was a different structure as in the java code. We try to mirror
the structure as far as possible.
