-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Expose WriteRequest.RefreshPolicy string representation #23106
Changes from 1 commit
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 |
---|---|---|
|
@@ -65,45 +65,70 @@ enum RefreshPolicy implements Writeable { | |
/** | ||
* Don't refresh after this request. The default. | ||
*/ | ||
NONE, | ||
NONE((byte) 0, Boolean.FALSE.toString()), | ||
/** | ||
* Force a refresh as part of this request. This refresh policy does not scale for high indexing or search throughput but is useful | ||
* to present a consistent view to for indices with very low traffic. And it is wonderful for tests! | ||
*/ | ||
IMMEDIATE, | ||
IMMEDIATE((byte) 1, Boolean.TRUE.toString()), | ||
/** | ||
* Leave this request open until a refresh has made the contents of this request visible to search. This refresh policy is | ||
* compatible with high indexing and search throughput but it causes the request to wait to reply until a refresh occurs. | ||
*/ | ||
WAIT_UNTIL; | ||
WAIT_UNTIL((byte) 2, "wait_for"); | ||
|
||
private final byte id; | ||
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. can we remove the id member and stick with the enum ordinal like we were doing before? 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. Yes we can :( |
||
private final String value; | ||
|
||
RefreshPolicy(byte id, String value) { | ||
this.id = id; | ||
this.value = value; | ||
} | ||
|
||
public byte getId() { | ||
return id; | ||
} | ||
|
||
public String getValue() { | ||
return value; | ||
} | ||
|
||
/** | ||
* Parse the string representation of a refresh policy, usually from a request parameter. | ||
*/ | ||
public static RefreshPolicy parse(String string) { | ||
switch (string) { | ||
case "false": | ||
return NONE; | ||
/* | ||
* Empty string is IMMEDIATE because that makes "POST /test/test/1?refresh" perform a refresh which reads well and is what folks | ||
* are used to. | ||
*/ | ||
case "": | ||
case "true": | ||
public static RefreshPolicy parse(String value) { | ||
for (RefreshPolicy policy : values()) { | ||
if (policy.getValue().equals(value)) { | ||
return policy; | ||
} | ||
} | ||
if ("".equals(value)) { | ||
// Empty string is IMMEDIATE because that makes "POST /test/test/1?refresh" perform | ||
// a refresh which reads well and is what folks are used to. | ||
return IMMEDIATE; | ||
case "wait_for": | ||
return WAIT_UNTIL; | ||
} | ||
throw new IllegalArgumentException("Unknown value for refresh: [" + string + "]."); | ||
throw new IllegalArgumentException("Unknown value for refresh: [" + value + "]."); | ||
} | ||
|
||
/** | ||
* Returns the {@link RefreshPolicy} associated to the given id. | ||
*/ | ||
public static RefreshPolicy fromId(byte id) { | ||
for (RefreshPolicy policy : values()) { | ||
if (policy.getId() == id) { | ||
return policy; | ||
} | ||
} | ||
throw new IllegalArgumentException("No cluster block level matching [" + id + "]"); | ||
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. is this a cluster block level? 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. It's a wonderful copy paste |
||
} | ||
|
||
public static RefreshPolicy readFrom(StreamInput in) throws IOException { | ||
return RefreshPolicy.values()[in.readByte()]; | ||
return fromId(in.readByte()); | ||
} | ||
|
||
@Override | ||
public void writeTo(StreamOutput out) throws IOException { | ||
out.writeByte((byte) ordinal()); | ||
out.writeByte(getId()); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
/* | ||
* Licensed to Elasticsearch under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
package org.elasticsearch.action.support; | ||
|
||
import org.elasticsearch.common.io.stream.BytesStreamOutput; | ||
import org.elasticsearch.common.io.stream.StreamInput; | ||
import org.elasticsearch.test.ESTestCase; | ||
|
||
import java.io.IOException; | ||
|
||
public class RefreshPolicyTests extends ESTestCase { | ||
|
||
public void testSerialization() throws IOException { | ||
final WriteRequest.RefreshPolicy refreshPolicy = randomFrom(WriteRequest.RefreshPolicy.values()); | ||
try (BytesStreamOutput out = new BytesStreamOutput()) { | ||
refreshPolicy.writeTo(out); | ||
try (StreamInput in = out.bytes().streamInput()) { | ||
WriteRequest.RefreshPolicy deserializedRefreshPolicy = WriteRequest.RefreshPolicy.readFrom(in); | ||
assertEquals(refreshPolicy, deserializedRefreshPolicy); | ||
} | ||
} | ||
} | ||
|
||
public void testFromId() throws IOException { | ||
final WriteRequest.RefreshPolicy refreshPolicy = randomFrom(WriteRequest.RefreshPolicy.values()); | ||
assertEquals(refreshPolicy, WriteRequest.RefreshPolicy.fromId(refreshPolicy.getId())); | ||
} | ||
|
||
public void testParse() throws IOException { | ||
final String refreshPolicyValue = randomFrom(WriteRequest.RefreshPolicy.values()).getValue(); | ||
assertEquals(refreshPolicyValue, WriteRequest.RefreshPolicy.parse(refreshPolicyValue).getValue()); | ||
} | ||
|
||
public void testParseEmpty() throws IOException { | ||
assertEquals(WriteRequest.RefreshPolicy.IMMEDIATE, WriteRequest.RefreshPolicy.parse("")); | ||
} | ||
|
||
public void testParseUnknown() throws IOException { | ||
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> WriteRequest.RefreshPolicy.parse("unknown")); | ||
assertEquals("Unknown value for refresh: [unknown].", e.getMessage()); | ||
} | ||
} |
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.
nit: I am wondering if using "true" and "false" would be more readable, simply because I always forget if it's lowercase or uppercase.