You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/06/05 18:02:55 UTC
[GitHub] [geode] jhutchison commented on a change in pull request #5216: Refactor set executor
jhutchison commented on a change in pull request #5216:
URL: https://github.com/apache/geode/pull/5216#discussion_r436079328
##########
File path: geode-redis/src/main/java/org/apache/geode/redis/internal/executor/string/SetExecutor.java
##########
@@ -68,72 +75,141 @@ private ByteArrayWrapper getValueToSet(List<byte[]> commandElems) {
return new ByteArrayWrapper(value);
}
+ private SetOptions parseOptionalParameters(List<byte[]> optionalParameterBytes)
+ throws IllegalArgumentException {
- private SetOptions parseCommandElems(List<byte[]> commandElems) throws IllegalArgumentException {
boolean keepTTL = false;
SetOptions.Exists existsOption = SetOptions.Exists.NONE;
long expiration = 0L;
- for (int i = 3; i < commandElems.size(); i++) {
- String current_arg = Coder.bytesToString(commandElems.get(i)).toUpperCase();
- switch (current_arg) {
- case "KEEPTTL":
- keepTTL = true;
- break;
- case "EX":
- if (expiration != 0) {
- throw new IllegalArgumentException(ERROR_SYNTAX);
- }
- i++;
- expiration = parseExpirationTime(i, commandElems);
- expiration = SECONDS.toMillis(expiration);
- break;
- case "PX":
- if (expiration != 0) {
- throw new IllegalArgumentException(ERROR_SYNTAX);
- }
- i++;
- expiration = parseExpirationTime(i, commandElems);
- break;
- case "NX":
- if (existsOption != SetOptions.Exists.NONE) {
- throw new IllegalArgumentException(ERROR_SYNTAX);
- }
- existsOption = SetOptions.Exists.NX;
- break;
- case "XX":
- if (existsOption != SetOptions.Exists.NONE) {
- throw new IllegalArgumentException(ERROR_SYNTAX);
- }
- existsOption = SetOptions.Exists.XX;
- break;
- default:
- throw new IllegalArgumentException(ERROR_SYNTAX);
+ List<String> optionalParametersStrings =
+ optionalParameterBytes.stream()
+ .map(item -> Coder.bytesToString(item).toUpperCase())
+ .collect(Collectors.toList());
+
+ throwExceptionIfUnknownParameter(optionalParametersStrings);
+ throwExceptionIfIncompatableParamaterOptions(optionalParametersStrings);
+
+ if (optionalParametersStrings.contains("KEEPTL")) {
+ keepTTL = true;
+ }
+
+ if (optionalParametersStrings.contains("PX")) {
+ int index = optionalParametersStrings.indexOf("PX");
+
+ if (optionalParametersStrings.size() <= index + 1) {
+ throw new IllegalArgumentException(ERROR_SYNTAX);
+ }
+
+ String expirationTime = optionalParametersStrings.get(index + 1);
+ expiration = parseExpirationTime("PX", expirationTime);
+
+ } else if (optionalParametersStrings.contains("EX")) {
+ int index = optionalParametersStrings.indexOf("EX");
+
+ if (optionalParametersStrings.size() <= index + 1) {
+ throw new IllegalArgumentException(ERROR_SYNTAX);
}
+
+ String expirationTime = optionalParametersStrings.get(index + 1);
+ expiration = parseExpirationTime("EX", expirationTime);
+ }
+
+ if (optionalParametersStrings.contains("NX")) {
+ existsOption = SetOptions.Exists.NX;
+ } else if (optionalParametersStrings.contains("XX")) {
+ existsOption = SetOptions.Exists.XX;
}
return new SetOptions(existsOption, expiration, keepTTL);
}
- private long parseExpirationTime(int index, List<byte[]> commandElems)
- throws IllegalArgumentException {
- String expirationString;
- try {
- expirationString = Coder.bytesToString(commandElems.get(index));
- } catch (IndexOutOfBoundsException e) {
+ private void throwExceptionIfUnknownParameter(List<String> optionalParameters) {
+ List<String> validOptionalParamaters = Arrays.asList("EX", "PX", "NX", "XX", "KEEPTL");
+
+ List<String> parametersInQuestion =
+ optionalParameters
+ .stream()
+ .filter(parameter -> (!validOptionalParamaters.contains(parameter)))
+ .collect(Collectors.toList());
+
+ parametersInQuestion.forEach(parameter -> throwErrorIfNotANumberInExpectedPosition(
+ parameter,
+ optionalParameters));
+ }
+
+
+ private void throwErrorIfNotANumberInExpectedPosition(
+ String parameter,
+ List<String> optionalParameters) {
+
+ if (previousOptionIsValidAndExpectsANumber(parameter, optionalParameters)) {
+ convertToLongOrThrowException(parameter);
+ } else {
throw new IllegalArgumentException(ERROR_SYNTAX);
}
+ }
+
+
+ private boolean previousOptionIsValidAndExpectsANumber(String paramter,
+ List<String> optionalParameters) {
+ List<String> validParamaters = Arrays.asList("EX", "PX");
+
+ if (optionalParameters.size() < 2) {
+ return false;
+ }
+
+ int indexOfParameter = optionalParameters.indexOf(paramter);
+ String previousParameter = optionalParameters.get(indexOfParameter - 1);
+
+ return validParamaters.contains(previousParameter);
+ }
+
+
+ private void throwExceptionIfIncompatableParamaterOptions(List<String> passedParametersStrings) {
+
+ if (passedParametersStrings.contains("PX")
+ && passedParametersStrings.contains("EX")) {
+ throw new IllegalArgumentException(ERROR_SYNTAX);
+ }
+
+ if (passedParametersStrings.contains("XX")
+ && passedParametersStrings.contains("NX")) {
+ throw new IllegalArgumentException(ERROR_SYNTAX);
+ }
+ }
+
+ private long parseExpirationTime(String expirationParameter, String expirationTimeString)
+ throws IllegalArgumentException {
+
+ long expirationTimeLong =
+ convertToLongOrThrowException(expirationTimeString);
+
+ isExpirationTimeLegal(expirationTimeLong);
+
+ if (expirationParameter.equals("EX")) {
+ expirationTimeLong *= 1000;
Review comment:
makes sense- rebase oversight. done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org