-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
API versioning #3155
API versioning #3155
Changes from 6 commits
d1193b0
da69178
9da2555
a122f4f
f51bc3a
e3deece
f4661e0
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 |
---|---|---|
|
@@ -114,10 +114,42 @@ parseRippleLibSeed(Json::Value const& params); | |
std::pair<PublicKey, SecretKey> | ||
keypairForSignature(Json::Value const& params, Json::Value& error); | ||
|
||
/** | ||
* API version numbers used in API version 1 | ||
*/ | ||
extern beast::SemanticVersion const firstVersion; | ||
extern beast::SemanticVersion const goodVersion; | ||
extern beast::SemanticVersion const lastVersion; | ||
|
||
/** | ||
* API version numbers used in later API versions | ||
* | ||
* Requests with a version number in the range | ||
* [ApiMinimumSupportedVersion, ApiMaximumSupportedVersion] | ||
* are supported. | ||
* | ||
* Network Requests without explicit version numbers use | ||
* APIVersionIfUnspecified. APIVersionIfUnspecified is 1, | ||
* because all the RPC requests with a version >= 2 must | ||
* explicitly specify the version in the requests. | ||
* Note that APIVersionIfUnspecified will be lower than | ||
* ApiMinimumSupportedVersion when we stop supporting API | ||
* version 1. | ||
* | ||
* Command line Requests use ApiMaximumSupportedVersion. | ||
*/ | ||
|
||
constexpr unsigned int APIInvalidVersion = 0; | ||
constexpr unsigned int APIVersionIfUnspecified = 1; | ||
constexpr unsigned int ApiMinimumSupportedVersion = 1; | ||
constexpr unsigned int ApiMaximumSupportedVersion = 1; | ||
constexpr unsigned int APINumberVersionSupported = ApiMaximumSupportedVersion - | ||
ApiMinimumSupportedVersion + 1; | ||
|
||
static_assert (ApiMinimumSupportedVersion >= APIVersionIfUnspecified); | ||
static_assert (ApiMaximumSupportedVersion >= ApiMinimumSupportedVersion); | ||
|
||
|
||
nbougalis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
template <class Object> | ||
void | ||
setVersion(Object& parent) | ||
|
@@ -131,6 +163,20 @@ setVersion(Object& parent) | |
std::pair<RPC::Status, LedgerEntryType> | ||
chooseLedgerEntryType(Json::Value const& params); | ||
|
||
/** | ||
* Retrieve the api version number from the json value | ||
* | ||
* Note that APIInvalidVersion will be returned if | ||
* 1) the version number field has a wrong format | ||
* 2) the version number retrieved is out of the supported range | ||
* 3) the version number is unspecified and | ||
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. seems like case 3 should be covered by the static checks that @nbougalis suggested above 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. You are right. But I think we need both the static check (checking we don't make a mistake in coding) and the check in the function (checking network requests are acceptable). |
||
* APIVersionIfUnspecified is out of the supported range | ||
* | ||
* @param value a Json value that may or may not specifies | ||
* the api version number | ||
* @return the api version number | ||
*/ | ||
unsigned int getAPIVersionNumber(const Json::Value & value); | ||
} // RPC | ||
} // ripple | ||
|
||
|
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.
the json arrays support range-based iteration, FWIW. Furthermore, since
isMember
already checksisObject
, you can just just callstd::for_each(jvRequest.begin(), jvRequest.end(), insert_api_version);
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.
Thanks, I changed to
std::for_each(jvRequest.begin(), jvRequest.end(), insert_api_version);
I modified
insert_api_version()
toinclude isObject()
. I think that is still needed. Otherwise,Value::operator[]
could be called on a non-object.