[java-plugin-shibd] branch main updated: Adjust various aspects of storage flow in prep for further work.
Codeberg
noreply at shibboleth.net
Fri Nov 21 20:40:34 UTC 2025
This is an automated email from the git hooks/post-receive script.
codeberg pushed a commit to branch main
in repository java-plugin-shibd.
View the commit online:
https://codeberg.org/Shibboleth/java-plugin-shibd/commit/5a75fdd0bf4b5d31c33462d2733e77ed44e7f450
The following commit(s) were added to refs/heads/main by this push:
new 5a75fdd Adjust various aspects of storage flow in prep for further work.
5a75fdd is described below
commit 5a75fdd0bf4b5d31c33462d2733e77ed44e7f450
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Fri Nov 21 15:40:21 2025 -0500
Adjust various aspects of storage flow in prep for further work.
---
.../net/shibboleth/sp/flows/StorageFlowTest.java | 62 ++++++----
.../sp/profile/impl/DoStorageOperation.java | 128 ++++++++++++---------
2 files changed, 112 insertions(+), 78 deletions(-)
diff --git a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/StorageFlowTest.java b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/StorageFlowTest.java
index a01fe72..8cd53fd 100644
--- a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/StorageFlowTest.java
+++ b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/StorageFlowTest.java
@@ -83,14 +83,14 @@ public class StorageFlowTest extends AbstractSPFlowTest {
}
/**
- * Test invalid method.
+ * Test no operation.
*
* @throws IOException
*/
@Test
- public void invalidMethod() throws IOException {
+ public void noOperation() throws IOException {
setDefaultAuth();
- setRequest("FOO", new DDF(null));
+ setRequest("POST", new DDF(null));
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertFlowExecutionResult(result, FLOW_ID);
assertFlowExecutionOutcome(result.getOutcome());
@@ -98,7 +98,8 @@ public class StorageFlowTest extends AbstractSPFlowTest {
}
/**
- * Test wrap operation with no input.
+ * Test read operation with no record.
+ *
* @throws IOException
*/
@Test
@@ -106,9 +107,10 @@ public class StorageFlowTest extends AbstractSPFlowTest {
setDefaultAuth();
final DDF input = new DDF().structure();
+ input.addmember(DoStorageOperation.OP).string("R");
input.addmember(DoStorageOperation.CONTEXT).string(TEST_CONTEXT);
input.addmember(DoStorageOperation.KEY).string(TEST_KEY);
- setRequest("GET", input);
+ setRequest("POST", input);
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertFlowExecutionResult(result, FLOW_ID);
@@ -117,7 +119,7 @@ public class StorageFlowTest extends AbstractSPFlowTest {
}
/**
- * Test successful get.
+ * Test successful read.
*
* @throws IOException
*/
@@ -130,9 +132,10 @@ public class StorageFlowTest extends AbstractSPFlowTest {
getStorageService().create(AGENT_CONTEXT, TEST_KEY, TEST_VALUE, exp);
final DDF input = new DDF().structure();
+ input.addmember(DoStorageOperation.OP).string("R");
input.addmember(DoStorageOperation.CONTEXT).string(TEST_CONTEXT);
input.addmember(DoStorageOperation.KEY).string(TEST_KEY);
- setRequest("GET", input);
+ setRequest("POST", input);
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertFlowExecutionResult(result, FLOW_ID);
@@ -157,9 +160,10 @@ public class StorageFlowTest extends AbstractSPFlowTest {
getStorageService().create(AGENT_CONTEXT, TEST_KEY, TEST_VALUE, null);
final DDF input = new DDF().structure();
+ input.addmember(DoStorageOperation.OP).string("D");
input.addmember(DoStorageOperation.CONTEXT).string(TEST_CONTEXT);
input.addmember(DoStorageOperation.KEY).string(TEST_KEY + "2");
- setRequest("DELETE", input);
+ setRequest("POST", input);
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertFlowExecutionResult(result, FLOW_ID);
@@ -182,9 +186,10 @@ public class StorageFlowTest extends AbstractSPFlowTest {
getStorageService().create(AGENT_CONTEXT, TEST_KEY, TEST_VALUE, null);
final DDF input = new DDF().structure();
+ input.addmember(DoStorageOperation.OP).string("D");
input.addmember(DoStorageOperation.CONTEXT).string(TEST_CONTEXT);
input.addmember(DoStorageOperation.KEY).string(TEST_KEY);
- setRequest("DELETE", input);
+ setRequest("POST", input);
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertFlowExecutionResult(result, FLOW_ID);
@@ -205,10 +210,11 @@ public class StorageFlowTest extends AbstractSPFlowTest {
setDefaultAuth();
final DDF input = new DDF().structure();
+ input.addmember(DoStorageOperation.OP).string("C");
input.addmember(DoStorageOperation.CONTEXT).string(TEST_CONTEXT);
input.addmember(DoStorageOperation.KEY).string(TEST_KEY);
input.addmember(DoStorageOperation.VALUE).string(TEST_VALUE);
- setRequest("PUT", input);
+ setRequest("POST", input);
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertFlowExecutionResult(result, FLOW_ID);
@@ -234,10 +240,11 @@ public class StorageFlowTest extends AbstractSPFlowTest {
getStorageService().create(AGENT_CONTEXT, TEST_KEY, TEST_VALUE, null);
final DDF input = new DDF().structure();
+ input.addmember(DoStorageOperation.OP).string("C");
input.addmember(DoStorageOperation.CONTEXT).string(TEST_CONTEXT);
input.addmember(DoStorageOperation.KEY).string(TEST_KEY);
input.addmember(DoStorageOperation.VALUE).string(TEST_VALUE);
- setRequest("PUT", input);
+ setRequest("POST", input);
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertFlowExecutionResult(result, FLOW_ID);
@@ -246,20 +253,23 @@ public class StorageFlowTest extends AbstractSPFlowTest {
}
/**
- * Test successful update.
+ * Test successful update of expiration.
*
* @throws IOException
*/
@Test
- public void successUpdate() throws IOException {
+ public void successUpdateExp() throws IOException {
setDefaultAuth();
getStorageService().create(AGENT_CONTEXT, TEST_KEY, TEST_VALUE, null);
+ final Instant now = Instant.now();
+
final DDF input = new DDF().structure();
+ input.addmember(DoStorageOperation.OP).string("U");
input.addmember(DoStorageOperation.CONTEXT).string(TEST_CONTEXT);
input.addmember(DoStorageOperation.KEY).string(TEST_KEY);
- input.addmember(DoStorageOperation.VALUE).string("changed");
+ input.addmember(DoStorageOperation.EXP).longinteger(now.getEpochSecond() + 600);
setRequest("POST", input);
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
@@ -270,35 +280,40 @@ public class StorageFlowTest extends AbstractSPFlowTest {
final StorageRecord<?> record = getStorageService().read(AGENT_CONTEXT, TEST_KEY);
assert record != null;
- Assert.assertEquals(record.getVersion(), 2);
- Assert.assertEquals(record.getValue(), "changed");
+ Assert.assertEquals(record.getVersion(), 1);
+ Assert.assertEquals(record.getValue(), TEST_VALUE);
+ Assert.assertEquals(record.getExpiration(), (now.getEpochSecond() + 600) * 1000);
}
+
/**
- * Test successful update as a create.
+ * Test successful update.
*
* @throws IOException
*/
@Test
- public void successUpdateAsCreate() throws IOException {
+ public void successUpdate() throws IOException {
setDefaultAuth();
+ getStorageService().create(AGENT_CONTEXT, TEST_KEY, TEST_VALUE, null);
+
final DDF input = new DDF().structure();
+ input.addmember(DoStorageOperation.OP).string("U");
input.addmember(DoStorageOperation.CONTEXT).string(TEST_CONTEXT);
input.addmember(DoStorageOperation.KEY).string(TEST_KEY);
- input.addmember(DoStorageOperation.VALUE).string(TEST_VALUE);
+ input.addmember(DoStorageOperation.VALUE).string("changed");
setRequest("POST", input);
final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
assertFlowExecutionResult(result, FLOW_ID);
assertFlowExecutionOutcome(result.getOutcome());
-
+
assertOutputMessageSuccess(result);
final StorageRecord<?> record = getStorageService().read(AGENT_CONTEXT, TEST_KEY);
assert record != null;
- Assert.assertEquals(record.getVersion(), 1);
- Assert.assertEquals(record.getValue(), TEST_VALUE);
+ Assert.assertEquals(record.getVersion(), 2);
+ Assert.assertEquals(record.getValue(), "changed");
}
/**
@@ -313,6 +328,7 @@ public class StorageFlowTest extends AbstractSPFlowTest {
getStorageService().create(AGENT_CONTEXT, TEST_KEY, TEST_VALUE, null);
final DDF input = new DDF().structure();
+ input.addmember(DoStorageOperation.OP).string("U");
input.addmember(DoStorageOperation.CONTEXT).string(TEST_CONTEXT);
input.addmember(DoStorageOperation.KEY).string(TEST_KEY);
input.addmember(DoStorageOperation.VALUE).string("changed");
@@ -345,6 +361,7 @@ public class StorageFlowTest extends AbstractSPFlowTest {
getStorageService().create(AGENT_CONTEXT, TEST_KEY, TEST_VALUE, null);
final DDF input = new DDF().structure();
+ input.addmember(DoStorageOperation.OP).string("U");
input.addmember(DoStorageOperation.CONTEXT).string(TEST_CONTEXT);
input.addmember(DoStorageOperation.KEY).string(TEST_KEY);
input.addmember(DoStorageOperation.VALUE).string("changed");
@@ -373,6 +390,7 @@ public class StorageFlowTest extends AbstractSPFlowTest {
setDefaultAuth();
final DDF input = new DDF().structure();
+ input.addmember(DoStorageOperation.OP).string("U");
input.addmember(DoStorageOperation.CONTEXT).string(TEST_CONTEXT);
input.addmember(DoStorageOperation.KEY).string(TEST_KEY);
input.addmember(DoStorageOperation.VALUE).string("changed");
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoStorageOperation.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoStorageOperation.java
index 467e49d..606e12b 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoStorageOperation.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoStorageOperation.java
@@ -35,7 +35,6 @@ import net.shibboleth.shared.primitive.LoggerFactory;
import net.shibboleth.sp.Agent;
import net.shibboleth.sp.ddf.DDF;
import net.shibboleth.sp.profile.AbstractAgentAction;
-import jakarta.servlet.http.HttpServletRequest;
/**
* Action that implements a remote API for SP agents to access or alter {@link StorageService} records.
@@ -61,6 +60,9 @@ public class DoStorageOperation extends AbstractAgentAction {
/** Custom event for version mismatch on conditional update. */
@Nonnull @NotEmpty public static final String VERSION_MISMATCH = "VersionMismatch";
+ /** Input member for operation. */
+ @Nonnull @NotEmpty public static final String OP = "op";
+
/** Input member for storage context. */
@Nonnull @NotEmpty public static final String CONTEXT = "context";
@@ -132,18 +134,18 @@ public class DoStorageOperation extends AbstractAgentAction {
@Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
try {
- final HttpServletRequest request = ensureHttpServletRequest();
+ final String op = input.getmember(OP).string();
- if ("GET".equals(request.getMethod())) {
- doRead(profileRequestContext);
- } else if ("PUT".equals(request.getMethod())) {
+ if ("C".equals(op)) {
doCreate(profileRequestContext);
- } else if ("POST".equals(request.getMethod())) {
+ } else if ("R".equals(op)) {
+ doRead(profileRequestContext);
+ } else if ("U".equals(op)) {
doUpdate(profileRequestContext);
- } else if ("DELETE".equals(request.getMethod())) {
+ } else if ("D".equals(op)) {
doDelete(profileRequestContext);
} else {
- log.warn("{} Invalid method: {}", getLogPrefix(), request.getMethod());
+ log.warn("{} Invalid operation: {}", getLogPrefix(), op);
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
}
} catch (final IOException e) {
@@ -153,79 +155,78 @@ public class DoStorageOperation extends AbstractAgentAction {
}
/**
- * Perform read operation.
+ * Perform create operation.
*
* @param profileRequestContext profile request context
*
* @throws IOException if an error is raised
*/
- private void doRead(@Nonnull final ProfileRequestContext profileRequestContext) throws IOException {
+ private void doCreate(@Nonnull final ProfileRequestContext profileRequestContext) throws IOException {
String context = input.getmember(CONTEXT).string();
final String key = input.getmember(KEY).string();
- if (context == null || key == null) {
- log.warn("{} Context and key are required members for read operation", getLogPrefix());
+ final String value = input.getmember(VALUE).string();
+
+ if (context == null || key == null || value == null) {
+ log.warn("{} Context, key, and, value are required for create operation", getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
return;
}
-
+
// Decorate context with Agent ID for uniqueness.
context = ensureAgent().getId() + '!' + context;
- final StorageRecord<?> record = storageService.read(context, key);
- if (record != null) {
- final DDF output = new DDF().structure();
- output.addmember(VALUE).string(record.getValue());
- output.addmember(VERSION).longinteger(record.getVersion());
- final Long exp = record.getExpiration();
- if (exp != null) {
- // Convert back to seconds.
- output.addmember(EXP).longinteger(exp / 1000);
- }
- ensureAgentRequestContext().setOutput(output);
+ Long exp = input.getmember(EXP).longinteger();
+ if (exp != null) {
+ // Convert to ms.
+ exp *= 1000;
+ }
+
+ if (storageService.create(context, key, value, exp)) {
+ log.trace("{} Created record with context ({}), key ({}), expiration ({})", getLogPrefix(), context, key,
+ exp);
} else {
- log.trace("{} No record found with context ({}), key ({})", getLogPrefix(), context, key);
- ActionSupport.buildEvent(profileRequestContext, RECORD_NOT_FOUND);
+ log.warn("{} Record exists with context ({}), key ({})", getLogPrefix(), context, key);
+ ActionSupport.buildEvent(profileRequestContext, DUPLICATE_RECORD);
}
}
-
/**
- * Perform create operation.
+ * Perform read operation.
*
* @param profileRequestContext profile request context
*
* @throws IOException if an error is raised
*/
- private void doCreate(@Nonnull final ProfileRequestContext profileRequestContext) throws IOException {
+ private void doRead(@Nonnull final ProfileRequestContext profileRequestContext) throws IOException {
String context = input.getmember(CONTEXT).string();
final String key = input.getmember(KEY).string();
- final String value = input.getmember(VALUE).string();
-
- if (context == null || key == null || value == null) {
- log.warn("{} Context, key, and value are required members for create operation", getLogPrefix());
+ if (context == null || key == null) {
+ log.warn("{} Context and key are required for read operation", getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
return;
}
-
+
// Decorate context with Agent ID for uniqueness.
context = ensureAgent().getId() + '!' + context;
- Long exp = input.getmember(EXP).longinteger();
- if (exp != null) {
- // Convert to ms.
- exp *= 1000;
- }
-
- if (storageService.create(context, key, value, exp)) {
- log.trace("{} Created record with context ({}), key ({}), expiration ({})", getLogPrefix(), context, key,
- exp);
+ final StorageRecord<?> record = storageService.read(context, key);
+ if (record != null) {
+ final DDF output = new DDF().structure();
+ output.addmember(VALUE).string(record.getValue());
+ output.addmember(VERSION).longinteger(record.getVersion());
+ final Long exp = record.getExpiration();
+ if (exp != null) {
+ // Convert back to seconds.
+ output.addmember(EXP).longinteger(exp / 1000);
+ }
+ ensureAgentRequestContext().setOutput(output);
} else {
- log.warn("{} Record exists with context ({}), key ({})", getLogPrefix(), context, key);
- ActionSupport.buildEvent(profileRequestContext, DUPLICATE_RECORD);
+ log.trace("{} No record found with context ({}), key ({})", getLogPrefix(), context, key);
+ ActionSupport.buildEvent(profileRequestContext, RECORD_NOT_FOUND);
}
- }
+ }
/**
* Perform update operation.
@@ -239,23 +240,42 @@ public class DoStorageOperation extends AbstractAgentAction {
String context = input.getmember(CONTEXT).string();
final String key = input.getmember(KEY).string();
final String value = input.getmember(VALUE).string();
+ Long exp = input.getmember(EXP).longinteger();
- if (context == null || key == null || value == null) {
- log.warn("{} Context, key, and value are required members for update operation", getLogPrefix());
+ if (context == null || key == null) {
+ log.warn("{} Context and key are required for update operation", getLogPrefix());
+ ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
+ return;
+ } else if (value == null && exp == null) {
+ log.warn("{} Either value or expiration must be provided for update operation", getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
return;
}
+
// Decorate context with Agent ID for uniqueness.
context = ensureAgent().getId() + '!' + context;
- Long version = input.getmember(VERSION).longinteger();
- Long exp = input.getmember(EXP).longinteger();
if (exp != null) {
// Convert to ms.
exp *= 1000;
}
+ // No value means just update the expiration.
+ if (value == null) {
+ if (storageService.updateExpiration(context, key, exp)) {
+ log.trace("{} Updated expiration for context ({}), key ({})", getLogPrefix(), context, key);
+ final DDF output = new DDF().structure();
+ ensureAgentRequestContext().setOutput(output);
+ } else {
+ log.info("{} No record to update expiration with context ({}), key ({})", getLogPrefix(), context, key);
+ ActionSupport.buildEvent(profileRequestContext, RECORD_NOT_FOUND);
+ }
+ return;
+ }
+
+ Long version = input.getmember(VERSION).longinteger();
+
if (version != null) {
try {
version = storageService.updateWithVersion(version, context, key, value, exp);
@@ -278,12 +298,8 @@ public class DoStorageOperation extends AbstractAgentAction {
if (storageService.update(context, key, value, exp)) {
log.trace("{} Updated record with context ({}), key ({}), expiration ({})", getLogPrefix(), context,
key, exp);
- } else if (storageService.create(context, key, value, exp)) {
- log.trace("{} Created record with context ({}), key ({}), expiration ({})", getLogPrefix(), context,
- key, exp);
} else {
- log.warn("{} Failed to update or create record with context ({}), key ({})", getLogPrefix(), context,
- key);
+ log.warn("{} No record to update with context ({}), key ({})", getLogPrefix(), context, key);
ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
}
}
@@ -301,7 +317,7 @@ public class DoStorageOperation extends AbstractAgentAction {
String context = input.getmember(CONTEXT).string();
final String key = input.getmember(KEY).string();
if (context == null || key == null) {
- log.warn("{} Context and key are required members for delete operation", getLogPrefix());
+ log.warn("{} Context and key are required for delete operation", getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_MESSAGE);
return;
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list