-
Notifications
You must be signed in to change notification settings - Fork 69
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
Java: Add PING command #914
Changes from 1 commit
332d782
02b56fa
99b36e0
2f7760d
57a5115
40c9bf3
c302263
f78cdb6
0b2aa76
1b62d43
2cbf558
3b968f0
e68c8dd
df5d92e
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 |
---|---|---|
|
@@ -93,34 +93,40 @@ protected static CommandManager buildCommandManager(ChannelHandler channelHandle | |
} | ||
|
||
/** | ||
* Extracts the response from the Protobuf response and either throws an exception or returns the | ||
* appropriate response as an <code>Object</code>. | ||
* Extracts the value from a Redis response message and either throws an exception or returns the | ||
* value as an object of type {@link T}. If <code>isNullable</code>, than also returns <code>null | ||
* </code>. | ||
* | ||
* @param response Redis protobuf message | ||
* @return Response <code>Object</code> | ||
* @param classType Parameter {@link T} class type | ||
* @param isNullable Accepts null values in the protobuf message | ||
* @return Response as an object of type {@link T} or <code>null</code> | ||
* @param <T> return type | ||
* @throws RedisException on a type mismatch | ||
*/ | ||
private <T> T handleRedisResponse(Class classType, boolean isNullable, Response response) { | ||
Object value = | ||
new BaseCommandResponseResolver(RedisValueResolver::valueFromPointer).apply(response); | ||
if (isNullable && (value == null)) { | ||
return null; | ||
} | ||
if (classType.isInstance(value)) { | ||
return (T) value; | ||
} | ||
String className = value == null ? "null" : value.getClass().getSimpleName(); | ||
throw new RedisException( | ||
"Unexpected return type from Redis: got " | ||
+ className | ||
+ " expected " | ||
+ classType.toGenericString()); | ||
acarbonetto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
protected Object handleObjectResponse(Response response) { | ||
Yury-Fridlyand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// convert protobuf response into Object | ||
return new BaseCommandResponseResolver(RedisValueResolver::valueFromPointer).apply(response); | ||
return handleRedisResponse(Object.class, false, response); | ||
} | ||
|
||
/** | ||
* Extracts the response value from the Redis response and either throws an exception or returns | ||
* the value as a <code>String</code>. | ||
* | ||
* @param response Redis protobuf message | ||
* @return Response as a <code>String</code> | ||
* @throws RedisException if there's a type mismatch | ||
*/ | ||
protected String handleStringResponse(Response response) { | ||
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. not necessarily for this PR - this has a very similar structure to the handleArrayResponse. protected T handleResponse<T>(Response response) {
Object value = handleObjectResponse(response);
if (value instanceof T || value == null) {
return (T) value;
}
throw new RedisException(
"Unexpected return type from Redis: got "
+ value.getClass().getSimpleName()
+ " expected " + T.getType());
}
```
of course, `T.getType()` isn't really possible and this doesn't handle primitives well, but I believe this is possible, and will remove a lot of glue 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. good idea. I've updated to include the generic function - this will reduce code duplication. |
||
Object value = handleObjectResponse(response); | ||
if (value instanceof String || value == null) { | ||
return (String) value; | ||
} | ||
throw new RedisException( | ||
"Unexpected return type from Redis: got " | ||
+ value.getClass().getSimpleName() | ||
+ " expected String"); | ||
return handleRedisResponse(String.class, false, response); | ||
} | ||
|
||
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. Maybe we should make sections/regions - for handlers, for commands for all rest 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. Sure - but let's leave this for a future update. This will be useful when we have more than 3 commands. |
||
@Override | ||
|
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.
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 @shachlanAmazon for the idea
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.
why not private?
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.
updated in 2cbf558