Skip to content

Commit 30308b2

Browse files
committed
Make ImmutablePluginConfigVariable.refresh() a no-op instead of throwing.
The refresh() method semantically means "re-read from your backing store." For an immutable variable with no backing store, this is naturally a no-op. Only setValue() should throw since it attempts to mutate the value. This allows callers to safely call refresh() without needing to guard with isUpdatable() checks. Signed-off-by: David Venable <dlv@amazon.com>
1 parent 513dcfb commit 30308b2

3 files changed

Lines changed: 30 additions & 4 deletions

File tree

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/plugin/PluginConfigVariable.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,17 @@ public interface PluginConfigVariable {
3131
void setValue(Object updatedValue);
3232

3333
/**
34-
* Refresh the secret value on demand
34+
* Refresh the secret value on demand.
35+
* <p>
36+
* This call semantically means to refresh the value from the underlying data store. If that
37+
* is not supported an implementation should perform a no-op.
3538
*/
3639
void refresh();
3740

3841
/**
39-
* Returns if the variable is updatable.
42+
* Returns if the variable is updatable from Data Prepper itself. This indicates
43+
* that Data Prepper itself can update the actual value, not that the value itself can be
44+
* updated from another system.
4045
*
4146
* @return true if this variable is updatable, false otherwise
4247
*/

data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/VariableExpander.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,6 @@ public void setValue(Object updatedValue) {
9090

9191
@Override
9292
public void refresh() {
93-
// No-op as this is immutable
94-
throw new UnsupportedOperationException("ImmutablePluginConfigVariable doesn't support this operation");
9593
}
9694

9795
@Override

data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/VariableExpanderTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,29 @@ void testTranslateJsonParserWithSPluginConfigVariableValue_when_non_secret_forma
195195
assertFalse(pluginConfigVariableInstance.isUpdatable());
196196
}
197197

198+
@Test
199+
void testImmutablePluginConfigVariable_refresh_does_not_throw() throws IOException {
200+
final String testSecretReference = UUID.randomUUID().toString();
201+
final JsonParser jsonParser = JSON_FACTORY.createParser(String.format("\"%s\"", testSecretReference));
202+
jsonParser.nextToken();
203+
objectUnderTest = new VariableExpander(OBJECT_MAPPER, Set.of(pluginConfigValueTranslator));
204+
final Object actualResult = objectUnderTest.translate(jsonParser, PluginConfigVariable.class);
205+
PluginConfigVariable pluginConfigVariableInstance = (PluginConfigVariable) actualResult;
206+
pluginConfigVariableInstance.refresh();
207+
assertThat(pluginConfigVariableInstance.getValue().toString(), equalTo(testSecretReference));
208+
}
209+
210+
@Test
211+
void testImmutablePluginConfigVariable_setValue_throws() throws IOException {
212+
final String testSecretReference = UUID.randomUUID().toString();
213+
final JsonParser jsonParser = JSON_FACTORY.createParser(String.format("\"%s\"", testSecretReference));
214+
jsonParser.nextToken();
215+
objectUnderTest = new VariableExpander(OBJECT_MAPPER, Set.of(pluginConfigValueTranslator));
216+
final Object actualResult = objectUnderTest.translate(jsonParser, PluginConfigVariable.class);
217+
PluginConfigVariable pluginConfigVariableInstance = (PluginConfigVariable) actualResult;
218+
assertThrows(UnsupportedOperationException.class, () -> pluginConfigVariableInstance.setValue("new-value"));
219+
}
220+
198221
@Test
199222
void testTranslateJsonParserWithSPluginConfigVariableValue_translate_failure() throws IOException {
200223
final String testSecretKey = "testSecretKey";

0 commit comments

Comments
 (0)