Use ResourceFactory pattern

Signed-off-by: Craig Perkins <[email protected]>

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<Object> 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<? extends AbstractResource> getResourceClass() {
-        return SampleResource.class;
+    public ResourceFactory<? extends Resource> getResourceFactory() {
+        return new SampleResourceFactory();
     }
 
     @SuppressWarnings("unchecked")
     @Override
-    public void assignResourceSharingService(ResourceSharingService<? extends AbstractResource> service) {
+    public void assignResourceSharingService(ResourceSharingService<? extends Resource> service) {
         provider.set((ResourceSharingService<SampleResource>) service);
     }
 }

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;
     }

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;
 

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<SampleResource> {
+    @Override
+    public SampleResource createResource() {
+        return new SampleResource();
+    }
+}

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<T extends AbstractResource> implements ResourceSharingService<T> {
+public abstract class AbstractResourceSharingService<T extends Resource> implements ResourceSharingService<T> {
     protected final Client client;
     protected final String resourceIndex;
-    protected final Class<T> resourceClass;
+    protected final ResourceFactory<T> resourceFactory;
+    // protected final Class<T> resourceClass;
 
-    public AbstractResourceSharingService(Client client, String resourceIndex, Class<T> resourceClass) {
+    public AbstractResourceSharingService(Client client, String resourceIndex, ResourceFactory<T> resourceFactory) {
         this.client = client;
         this.resourceIndex = resourceIndex;
-        this.resourceClass = resourceClass;
+        this.resourceFactory = resourceFactory;
     }
 
-    protected T newResource() {
-        return AccessController.doPrivileged(new PrivilegedAction<T>() {
-            @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<T>() {
+    // @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<List<T>> 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<T> 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<T> getResourceListener
             ActionListener<GetResponse> getListener = new ActionListener<GetResponse>() {
                 @Override
                 public void onResponse(GetResponse getResponse) {
-                    T resource = newResource();
+                    T resource = resourceFactory.createResource();
                     resource.fromSource(getResponse.getId(), getResponse.getSourceAsMap());
                     getResourceListener.onResponse(resource);
                 }

spi/src/main/java/org/opensearch/security/spi/DefaultResourceSharingService.java

@@ -2,9 +2,9 @@
 
 import org.opensearch.client.Client;
 
-public class DefaultResourceSharingService<T extends AbstractResource> extends AbstractResourceSharingService<T> {
+public class DefaultResourceSharingService<T extends Resource> extends AbstractResourceSharingService<T> {
 
-    public DefaultResourceSharingService(Client client, String resourceIndex, Class<T> resourceClass) {
-        super(client, resourceIndex, resourceClass);
+    public DefaultResourceSharingService(Client client, String resourceIndex, ResourceFactory<T> resourceFactory) {
+        super(client, resourceIndex, resourceFactory);
     }
 }

spi/src/main/java/org/opensearch/security/spi/AbstractResource.java → 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;

spi/src/main/java/org/opensearch/security/spi/ResourceFactory.java

@@ -0,0 +1,5 @@
+package org.opensearch.security.spi;
+
+public interface ResourceFactory<T extends Resource> {
+    T createResource();
+}

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<? extends AbstractResource> getResourceClass();
+    ResourceFactory<? extends Resource> getResourceFactory();
 
-    void assignResourceSharingService(ResourceSharingService<? extends AbstractResource> service);
+    void assignResourceSharingService(ResourceSharingService<? extends Resource> service);
 }

spi/src/main/java/org/opensearch/security/spi/ResourceSharingService.java

@@ -4,7 +4,7 @@
 
 import org.opensearch.core.action.ActionListener;
 
-public interface ResourceSharingService<T extends AbstractResource> {
+public interface ResourceSharingService<T extends Resource> {
 
     void listResources(ActionListener<List<T>> listResourceListener);
 

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<T extends AbstractResource> extends ActionRequest {
+public class CreateResourceRequest<T extends Resource> extends ActionRequest {
 
     private final T resource;
 
@@ -45,7 +45,7 @@ public ActionRequestValidationException validate() {
         return null;
     }
 
-    public AbstractResource getResource() {
+    public Resource getResource() {
         return this.resource;
     }
 }

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<T extends AbstractResource> extends HandledTransportAction<
+public class CreateResourceTransportAction<T extends Resource> extends HandledTransportAction<
     CreateResourceRequest<T>,
     CreateResourceResponse> {
     private static final Logger log = LogManager.getLogger(CreateResourceTransportAction.class);
@@ -74,7 +74,7 @@ protected void doExecute(Task task, CreateResourceRequest<T> request, ActionList
 
     private void createResource(CreateResourceRequest<T> request, ActionListener<CreateResourceResponse> 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)

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<T extends AbstractResource> extends ActionRequest {
+public class UpdateResourceSharingRequest<T extends Resource> extends ActionRequest {
 
     private final String resourceId;
     private final ShareWith shareWith;

spi/src/main/java/org/opensearch/security/spi/actions/UpdateResourceSharingTransportAction.java

@@ -31,15 +31,15 @@
 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;
 
 /**
  * Transport action for CreateSampleResource.
  */
-public class UpdateResourceSharingTransportAction<T extends AbstractResource> extends HandledTransportAction<
+public class UpdateResourceSharingTransportAction<T extends Resource> extends HandledTransportAction<
     UpdateResourceSharingRequest<T>,
     UpdateResourceSharingResponse> {
     private static final Logger log = LogManager.getLogger(UpdateResourceSharingTransportAction.class);

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -1077,7 +1077,7 @@ public Collection<Object> createComponents(
             ResourceSharingService<?> resourceSharingService = new SecurityResourceSharingService<>(
                 localClient,
                 extension.getResourceIndex(),
-                extension.getResourceClass()
+                extension.getResourceFactory()
             );
             extension.assignResourceSharingService(resourceSharingService);
         }

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<T extends AbstractResource> extends AbstractResourceSharingService<T> {
-    public SecurityResourceSharingService(Client client, String resourceIndex, Class<T> resourceClass) {
-        super(client, resourceIndex, resourceClass);
+public class SecurityResourceSharingService<T extends Resource> extends AbstractResourceSharingService<T> {
+    public SecurityResourceSharingService(Client client, String resourceIndex, ResourceFactory<T> resourceFactory) {
+        super(client, resourceIndex, resourceFactory);
     }
 
     @SuppressWarnings("unchecked")
     @Override
     public void listResources(ActionListener<List<T>> 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<T> getResourceListener
             ActionListener<GetResponse> getListener = new ActionListener<GetResponse>() {
                 @Override
                 public void onResponse(GetResponse getResponse) {
-                    T resource = newResource();
+                    T resource = resourceFactory.createResource();
                     resource.fromSource(getResponse.getId(), getResponse.getSourceAsMap());
                     getResourceListener.onResponse(resource);
                 }