From c266a17cfaf9b1afb4731ed5b31d23366884a7b7 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 13 Dec 2024 10:35:59 -0500 Subject: [PATCH] Use ResourceFactory pattern Signed-off-by: Craig Perkins --- .../SampleExtensionPlugin.java | 12 +++--- .../list/ListSampleResourceResponse.java | 4 +- .../resource/SampleResource.java | 4 +- .../resource/SampleResourceFactory.java | 10 +++++ .../spi/AbstractResourceSharingService.java | 40 +++++++++---------- .../spi/DefaultResourceSharingService.java | 6 +-- .../{AbstractResource.java => Resource.java} | 4 +- .../security/spi/ResourceFactory.java | 5 +++ .../spi/ResourceSharingExtension.java | 4 +- .../security/spi/ResourceSharingService.java | 2 +- .../spi/actions/CreateResourceRequest.java | 6 +-- .../CreateResourceTransportAction.java | 6 +-- .../actions/UpdateResourceSharingRequest.java | 4 +- .../UpdateResourceSharingTransportAction.java | 4 +- .../security/OpenSearchSecurityPlugin.java | 2 +- .../SecurityResourceSharingService.java | 14 ++++--- 16 files changed, 72 insertions(+), 55 deletions(-) create mode 100644 sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResourceFactory.java rename spi/src/main/java/org/opensearch/security/spi/{AbstractResource.java => Resource.java} (88%) create mode 100644 spi/src/main/java/org/opensearch/security/spi/ResourceFactory.java diff --git a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java index 3b1e410ee3..5067708bc6 100644 --- a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java +++ b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java @@ -54,9 +54,11 @@ import org.opensearch.security.sampleextension.actions.update.UpdateSampleResourceRestAction; import org.opensearch.security.sampleextension.actions.update.UpdateSampleResourceTransportAction; import org.opensearch.security.sampleextension.resource.SampleResource; +import org.opensearch.security.sampleextension.resource.SampleResourceFactory; import org.opensearch.security.sampleextension.resource.SampleResourceSharingServiceProvider; -import org.opensearch.security.spi.AbstractResource; import org.opensearch.security.spi.DefaultResourceSharingService; +import org.opensearch.security.spi.Resource; +import org.opensearch.security.spi.ResourceFactory; import org.opensearch.security.spi.ResourceSharingExtension; import org.opensearch.security.spi.ResourceSharingService; import org.opensearch.threadpool.ThreadPool; @@ -93,7 +95,7 @@ public Collection createComponents( ) { this.client = client; if (provider.get() == null) { - provider.set(new DefaultResourceSharingService<>(client, RESOURCE_INDEX_NAME, SampleResource.class)); + provider.set(new DefaultResourceSharingService<>(client, RESOURCE_INDEX_NAME, new SampleResourceFactory())); } System.out.println("provider: " + provider); return List.of(provider); @@ -146,13 +148,13 @@ public String getResourceIndex() { } @Override - public Class getResourceClass() { - return SampleResource.class; + public ResourceFactory getResourceFactory() { + return new SampleResourceFactory(); } @SuppressWarnings("unchecked") @Override - public void assignResourceSharingService(ResourceSharingService service) { + public void assignResourceSharingService(ResourceSharingService service) { provider.set((ResourceSharingService) service); } } diff --git a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/actions/list/ListSampleResourceResponse.java b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/actions/list/ListSampleResourceResponse.java index c908970f67..7225ed835d 100644 --- a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/actions/list/ListSampleResourceResponse.java +++ b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/actions/list/ListSampleResourceResponse.java @@ -17,7 +17,7 @@ import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.security.sampleextension.resource.SampleResource; -import org.opensearch.security.spi.AbstractResource; +import org.opensearch.security.spi.Resource; /** * Response to a ListSampleResourceRequest @@ -51,7 +51,7 @@ public ListSampleResourceResponse(final StreamInput in) throws IOException { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); - builder.array("resources", (Object[]) resources.toArray(new AbstractResource[0])); + builder.array("resources", (Object[]) resources.toArray(new Resource[0])); builder.endObject(); return builder; } diff --git a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResource.java b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResource.java index 97038e5037..942449c8bb 100644 --- a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResource.java +++ b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResource.java @@ -6,9 +6,9 @@ import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.security.spi.AbstractResource; +import org.opensearch.security.spi.Resource; -public class SampleResource extends AbstractResource { +public class SampleResource extends Resource { private String name; diff --git a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResourceFactory.java b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResourceFactory.java new file mode 100644 index 0000000000..b902f38fb3 --- /dev/null +++ b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResourceFactory.java @@ -0,0 +1,10 @@ +package org.opensearch.security.sampleextension.resource; + +import org.opensearch.security.spi.ResourceFactory; + +public class SampleResourceFactory implements ResourceFactory { + @Override + public SampleResource createResource() { + return new SampleResource(); + } +} diff --git a/spi/src/main/java/org/opensearch/security/spi/AbstractResourceSharingService.java b/spi/src/main/java/org/opensearch/security/spi/AbstractResourceSharingService.java index c05b901b70..53e529936e 100644 --- a/spi/src/main/java/org/opensearch/security/spi/AbstractResourceSharingService.java +++ b/spi/src/main/java/org/opensearch/security/spi/AbstractResourceSharingService.java @@ -1,8 +1,5 @@ package org.opensearch.security.spi; -import java.lang.reflect.InvocationTargetException; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.List; @@ -18,34 +15,34 @@ import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; -public abstract class AbstractResourceSharingService implements ResourceSharingService { +public abstract class AbstractResourceSharingService implements ResourceSharingService { protected final Client client; protected final String resourceIndex; - protected final Class resourceClass; + protected final ResourceFactory resourceFactory; + // protected final Class resourceClass; - public AbstractResourceSharingService(Client client, String resourceIndex, Class resourceClass) { + public AbstractResourceSharingService(Client client, String resourceIndex, ResourceFactory resourceFactory) { this.client = client; this.resourceIndex = resourceIndex; - this.resourceClass = resourceClass; + this.resourceFactory = resourceFactory; } - protected T newResource() { - return AccessController.doPrivileged(new PrivilegedAction() { - @Override - public T run() { - try { - return resourceClass.getDeclaredConstructor().newInstance(); - } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { - throw new RuntimeException(e); - } - } - }); - } + // protected T newResource() { + // return AccessController.doPrivileged(new PrivilegedAction() { + // @Override + // public T run() { + // try { + // return resourceClass.getDeclaredConstructor().newInstance(); + // } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { + // throw new RuntimeException(e); + // } + // } + // }); + // } @SuppressWarnings("unchecked") @Override public void listResources(ActionListener> listResourceListener) { - T resource = newResource(); try (ThreadContext.StoredContext ignore = client.threadPool().getThreadContext().stashContext()) { SearchRequest sr = new SearchRequest(resourceIndex); SearchSourceBuilder matchAllQuery = new SearchSourceBuilder(); @@ -58,6 +55,7 @@ public void onResponse(SearchResponse searchResponse) { List resources = new ArrayList<>(); for (SearchHit hit : searchResponse.getHits().getHits()) { System.out.println("SearchHit: " + hit); + T resource = resourceFactory.createResource(); resource.fromSource(hit.getId(), hit.getSourceAsMap()); resources.add(resource); } @@ -83,7 +81,7 @@ public void getResource(String resourceId, ActionListener getResourceListener ActionListener getListener = new ActionListener() { @Override public void onResponse(GetResponse getResponse) { - T resource = newResource(); + T resource = resourceFactory.createResource(); resource.fromSource(getResponse.getId(), getResponse.getSourceAsMap()); getResourceListener.onResponse(resource); } diff --git a/spi/src/main/java/org/opensearch/security/spi/DefaultResourceSharingService.java b/spi/src/main/java/org/opensearch/security/spi/DefaultResourceSharingService.java index 9c17707a32..faade29b3b 100644 --- a/spi/src/main/java/org/opensearch/security/spi/DefaultResourceSharingService.java +++ b/spi/src/main/java/org/opensearch/security/spi/DefaultResourceSharingService.java @@ -2,9 +2,9 @@ import org.opensearch.client.Client; -public class DefaultResourceSharingService extends AbstractResourceSharingService { +public class DefaultResourceSharingService extends AbstractResourceSharingService { - public DefaultResourceSharingService(Client client, String resourceIndex, Class resourceClass) { - super(client, resourceIndex, resourceClass); + public DefaultResourceSharingService(Client client, String resourceIndex, ResourceFactory resourceFactory) { + super(client, resourceIndex, resourceFactory); } } diff --git a/spi/src/main/java/org/opensearch/security/spi/AbstractResource.java b/spi/src/main/java/org/opensearch/security/spi/Resource.java similarity index 88% rename from spi/src/main/java/org/opensearch/security/spi/AbstractResource.java rename to spi/src/main/java/org/opensearch/security/spi/Resource.java index 7fe36bb92d..8104ecc418 100644 --- a/spi/src/main/java/org/opensearch/security/spi/AbstractResource.java +++ b/spi/src/main/java/org/opensearch/security/spi/Resource.java @@ -6,11 +6,11 @@ import org.opensearch.core.common.io.stream.NamedWriteable; import org.opensearch.core.xcontent.ToXContentFragment; -public abstract class AbstractResource implements NamedWriteable, ToXContentFragment { +public abstract class Resource implements NamedWriteable, ToXContentFragment { protected ResourceUser resourceUser; protected String resourceId; - public AbstractResource() {} + public Resource() {} public String getResourceId() { return resourceId; diff --git a/spi/src/main/java/org/opensearch/security/spi/ResourceFactory.java b/spi/src/main/java/org/opensearch/security/spi/ResourceFactory.java new file mode 100644 index 0000000000..ab86c7a819 --- /dev/null +++ b/spi/src/main/java/org/opensearch/security/spi/ResourceFactory.java @@ -0,0 +1,5 @@ +package org.opensearch.security.spi; + +public interface ResourceFactory { + T createResource(); +} diff --git a/spi/src/main/java/org/opensearch/security/spi/ResourceSharingExtension.java b/spi/src/main/java/org/opensearch/security/spi/ResourceSharingExtension.java index eb95a9c236..85cb73dcb5 100644 --- a/spi/src/main/java/org/opensearch/security/spi/ResourceSharingExtension.java +++ b/spi/src/main/java/org/opensearch/security/spi/ResourceSharingExtension.java @@ -25,7 +25,7 @@ public interface ResourceSharingExtension { /** * @return The class corresponding to this resource */ - Class getResourceClass(); + ResourceFactory getResourceFactory(); - void assignResourceSharingService(ResourceSharingService service); + void assignResourceSharingService(ResourceSharingService service); } diff --git a/spi/src/main/java/org/opensearch/security/spi/ResourceSharingService.java b/spi/src/main/java/org/opensearch/security/spi/ResourceSharingService.java index c73e47a4c7..6faabaeee7 100644 --- a/spi/src/main/java/org/opensearch/security/spi/ResourceSharingService.java +++ b/spi/src/main/java/org/opensearch/security/spi/ResourceSharingService.java @@ -4,7 +4,7 @@ import org.opensearch.core.action.ActionListener; -public interface ResourceSharingService { +public interface ResourceSharingService { void listResources(ActionListener> listResourceListener); diff --git a/spi/src/main/java/org/opensearch/security/spi/actions/CreateResourceRequest.java b/spi/src/main/java/org/opensearch/security/spi/actions/CreateResourceRequest.java index 6ee2a67c3d..e930cf1ecf 100644 --- a/spi/src/main/java/org/opensearch/security/spi/actions/CreateResourceRequest.java +++ b/spi/src/main/java/org/opensearch/security/spi/actions/CreateResourceRequest.java @@ -15,12 +15,12 @@ import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.common.io.stream.Writeable; -import org.opensearch.security.spi.AbstractResource; +import org.opensearch.security.spi.Resource; /** * Request object for CreateSampleResource transport action */ -public class CreateResourceRequest extends ActionRequest { +public class CreateResourceRequest extends ActionRequest { private final T resource; @@ -45,7 +45,7 @@ public ActionRequestValidationException validate() { return null; } - public AbstractResource getResource() { + public Resource getResource() { return this.resource; } } diff --git a/spi/src/main/java/org/opensearch/security/spi/actions/CreateResourceTransportAction.java b/spi/src/main/java/org/opensearch/security/spi/actions/CreateResourceTransportAction.java index 4df474019a..ede88c5326 100644 --- a/spi/src/main/java/org/opensearch/security/spi/actions/CreateResourceTransportAction.java +++ b/spi/src/main/java/org/opensearch/security/spi/actions/CreateResourceTransportAction.java @@ -25,7 +25,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.io.stream.Writeable; import org.opensearch.core.xcontent.ToXContent; -import org.opensearch.security.spi.AbstractResource; +import org.opensearch.security.spi.Resource; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; @@ -34,7 +34,7 @@ /** * Transport action for CreateSampleResource. */ -public class CreateResourceTransportAction extends HandledTransportAction< +public class CreateResourceTransportAction extends HandledTransportAction< CreateResourceRequest, CreateResourceResponse> { private static final Logger log = LogManager.getLogger(CreateResourceTransportAction.class); @@ -74,7 +74,7 @@ protected void doExecute(Task task, CreateResourceRequest request, ActionList private void createResource(CreateResourceRequest request, ActionListener listener) { log.warn("Sample name: " + request.getResource()); - AbstractResource sample = request.getResource(); + Resource sample = request.getResource(); try { IndexRequest ir = nodeClient.prepareIndex(resourceIndex) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) diff --git a/spi/src/main/java/org/opensearch/security/spi/actions/UpdateResourceSharingRequest.java b/spi/src/main/java/org/opensearch/security/spi/actions/UpdateResourceSharingRequest.java index e0cfca44fd..c658b34430 100644 --- a/spi/src/main/java/org/opensearch/security/spi/actions/UpdateResourceSharingRequest.java +++ b/spi/src/main/java/org/opensearch/security/spi/actions/UpdateResourceSharingRequest.java @@ -14,13 +14,13 @@ import org.opensearch.action.ActionRequestValidationException; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.security.spi.AbstractResource; +import org.opensearch.security.spi.Resource; import org.opensearch.security.spi.ShareWith; /** * Request object for CreateSampleResource transport action */ -public class UpdateResourceSharingRequest extends ActionRequest { +public class UpdateResourceSharingRequest extends ActionRequest { private final String resourceId; private final ShareWith shareWith; diff --git a/spi/src/main/java/org/opensearch/security/spi/actions/UpdateResourceSharingTransportAction.java b/spi/src/main/java/org/opensearch/security/spi/actions/UpdateResourceSharingTransportAction.java index a4b15bdf81..35cd9105e4 100644 --- a/spi/src/main/java/org/opensearch/security/spi/actions/UpdateResourceSharingTransportAction.java +++ b/spi/src/main/java/org/opensearch/security/spi/actions/UpdateResourceSharingTransportAction.java @@ -31,7 +31,7 @@ import org.opensearch.index.query.QueryBuilders; import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; -import org.opensearch.security.spi.AbstractResource; +import org.opensearch.security.spi.Resource; import org.opensearch.security.spi.ShareWith; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; @@ -39,7 +39,7 @@ /** * Transport action for CreateSampleResource. */ -public class UpdateResourceSharingTransportAction extends HandledTransportAction< +public class UpdateResourceSharingTransportAction extends HandledTransportAction< UpdateResourceSharingRequest, UpdateResourceSharingResponse> { private static final Logger log = LogManager.getLogger(UpdateResourceSharingTransportAction.class); diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 7d07a7f377..f0b9bab4c0 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1077,7 +1077,7 @@ public Collection createComponents( ResourceSharingService resourceSharingService = new SecurityResourceSharingService<>( localClient, extension.getResourceIndex(), - extension.getResourceClass() + extension.getResourceFactory() ); extension.assignResourceSharingService(resourceSharingService); } diff --git a/src/main/java/org/opensearch/security/resource/SecurityResourceSharingService.java b/src/main/java/org/opensearch/security/resource/SecurityResourceSharingService.java index b0e29ad12b..f649e92e0e 100644 --- a/src/main/java/org/opensearch/security/resource/SecurityResourceSharingService.java +++ b/src/main/java/org/opensearch/security/resource/SecurityResourceSharingService.java @@ -29,22 +29,22 @@ import org.opensearch.index.query.QueryBuilders; import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; -import org.opensearch.security.spi.AbstractResource; import org.opensearch.security.spi.AbstractResourceSharingService; +import org.opensearch.security.spi.Resource; +import org.opensearch.security.spi.ResourceFactory; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.User; import static org.opensearch.security.resource.ResourceSharingListener.RESOURCE_SHARING_INDEX; -public class SecurityResourceSharingService extends AbstractResourceSharingService { - public SecurityResourceSharingService(Client client, String resourceIndex, Class resourceClass) { - super(client, resourceIndex, resourceClass); +public class SecurityResourceSharingService extends AbstractResourceSharingService { + public SecurityResourceSharingService(Client client, String resourceIndex, ResourceFactory resourceFactory) { + super(client, resourceIndex, resourceFactory); } @SuppressWarnings("unchecked") @Override public void listResources(ActionListener> listResourceListener) { - T resource = newResource(); User authenticatedUser = client.threadPool().getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); try (ThreadContext.StoredContext ignore = client.threadPool().getThreadContext().stashContext()) { SearchRequest rsr = new SearchRequest(RESOURCE_SHARING_INDEX); @@ -101,6 +101,8 @@ public void onResponse(MultiGetResponse response) { if (singleResponse != null && !singleResponse.isFailed()) { GetResponse singleGetResponse = singleResponse.getResponse(); if (singleGetResponse.isExists() && !singleGetResponse.isSourceEmpty()) { + // TODO Is there a better way to create this instance of a generic w/o using reflection + T resource = resourceFactory.createResource(); resource.fromSource(singleGetResponse.getId(), singleGetResponse.getSourceAsMap()); resources.add(resource); } else { @@ -172,7 +174,7 @@ public void getResource(String resourceId, ActionListener getResourceListener ActionListener getListener = new ActionListener() { @Override public void onResponse(GetResponse getResponse) { - T resource = newResource(); + T resource = resourceFactory.createResource(); resource.fromSource(getResponse.getId(), getResponse.getSourceAsMap()); getResourceListener.onResponse(resource); }