Revert "[Upd] Workaround to fix memory leakage of stream resources"

This reverts commit 885a7ba.
kbss-cvut · Feb 12, 2023 · 3d19848 · 3d19848
1 parent c8d0283
commit 3d19848
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 54 deletions.
diff --git a/s-pipes-core/src/main/java/cz/cvut/spipes/registry/StreamResourceRegistry.java b/s-pipes-core/src/main/java/cz/cvut/spipes/registry/StreamResourceRegistry.java
@@ -1,18 +1,19 @@
 package cz.cvut.spipes.registry;
 
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.lang.ref.WeakReference;
-import java.util.*;
-
 public class StreamResourceRegistry {
     private static final Logger LOG = LoggerFactory.getLogger(StreamResourceRegistry.class);
 
     private static StreamResourceRegistry instance;
     private Set<String> resourcePrefixMap = new HashSet<>();
     private static final String PERSISTENT_CONTEXT_PREFIX = "http://onto.fel.cvut.cz/resources/";
-    private Map<String, WeakReference<StreamResource>> id2resourcesMap = new HashMap<>();
+    private Map<String, StreamResource> id2resourcesMap = new HashMap<>();
 
     private StreamResourceRegistry() {
     }
@@ -35,7 +36,7 @@ public String getPERSISTENT_CONTEXT_PREFIX() {
     }
 
     public StreamResource getResourceById(String id) {
-        return id2resourcesMap.get(id).get();
+        return id2resourcesMap.get(id);
     }
 
     public StreamResource getResourceByUrl(String url) {
@@ -48,20 +49,19 @@ public StreamResource getResourceByUrl(String url) {
                 .findAny().map(p -> url.substring(p.length()))
                 .orElse(null);
         LOG.debug("- found {}", id);
-        StreamResource res = id2resourcesMap.get(id).get();
+        StreamResource res = id2resourcesMap.get(id);
         if (res == null) {
             return null;
         }
         return new StringStreamResource(url, res.getContent(), res.getContentType()); //TODO remove
     }
 
-    public StreamResource registerResource(String id, byte[] content, String contentType) {
+    public void registerResource(String id, byte[] content, String contentType) {
         LOG.debug("Registering resource with id {}", id);
         StreamResource res = new StringStreamResource(id, content, contentType);
-        id2resourcesMap.put(id, new WeakReference<>(res));
+        id2resourcesMap.put(id, res);
         if (LOG.isTraceEnabled()) {
             LOG.trace("Resource map content after the registration: {}", id2resourcesMap);
         }
-        return res;
     }
 }
diff --git a/s-pipes-web/src/main/java/cz/cvut/spipes/rest/SPipesServiceController.java b/s-pipes-web/src/main/java/cz/cvut/spipes/rest/SPipesServiceController.java
@@ -31,8 +31,10 @@
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.net.URL;
-import java.util.*;
-import java.util.stream.Collectors;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 
 @RestController
 @EnableWebMvc
@@ -143,24 +145,11 @@ public Model processServiceGetRequest(@RequestParam MultiValueMap<String, String
     public Model processServicePostRequest(@RequestParam MultiValueMap<String, String> parameters,
                                            @RequestParam("files") MultipartFile[] files) {
 
-
-        List<StreamResourceDTO> newStreamResources = new LinkedList<>();
         MultiValueMap<String, String> newParameters =
-            new MultipartFileResourceResolver(resourceRegisterHelper).resolveResources(
-                parameters,
-                files,
-                newStreamResources
-            );
+            new MultipartFileResourceResolver(resourceRegisterHelper).resolveResources(parameters, files);
 
         LOG.info("Processing service POST request, with {} multipart file(s).", files.length);
-        Model model =  runService(ModelFactory.createDefaultModel(), newParameters);
-        if (! newStreamResources.isEmpty()) {
-            LOG.info(
-                "Loosing reference to stream resources: " +
-                newStreamResources.stream().map(StreamResourceDTO::getId).collect(Collectors.toList())
-            );
-        }
-        return model;
+        return runService(ModelFactory.createDefaultModel(), newParameters);
     }
 
     @ExceptionHandler

diff --git a/s-pipes-web/src/main/java/cz/cvut/spipes/rest/StreamResourceDTO.java b/s-pipes-web/src/main/java/cz/cvut/spipes/rest/StreamResourceDTO.java
@@ -1,12 +1,9 @@
 package cz.cvut.spipes.rest;
 
-import cz.cvut.spipes.registry.StreamResource;
-
 public class StreamResourceDTO {
     String id;
     String persistentUri;
     String alternativeUri;
-    StreamResource resource;
 
     public StreamResourceDTO(String id,
                              String persistentUriPrefix,
@@ -27,8 +24,4 @@ public String getPersistentUri() {
     public String getAlternativeUri() {
         return alternativeUri;
     }
-
-    public void attachStreamResource(StreamResource resource) {
-        this.resource = resource;
-    }
 }
diff --git a/s-pipes-web/src/main/java/cz/cvut/spipes/rest/util/MultipartFileResourceResolver.java b/s-pipes-web/src/main/java/cz/cvut/spipes/rest/util/MultipartFileResourceResolver.java
@@ -12,7 +12,6 @@
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.List;
 import java.util.Optional;
 
 /**
@@ -35,26 +34,19 @@ public MultipartFileResourceResolver(ResourceRegisterHelper resourceRegisterHelp
      * are prefixed by symbol "@" followed by name of the file. Referenced file are registered
      * as {@code cz.cvut.spipes.registry.StreamResource} which produces URI that is used to replace the original
      * parameter value.
-     * <p>
+     *
      * Example: "myParam"="@input.csv"
-     * <p>
+     *
      * Within the example, "input.csv" is name of file searched within the {@code files}. It such file is found,
      * it is registered as {@code cz.cvut.spipes.registry.StreamResource} whose generated URL is replaced
      * back to "myParam". Then the parameter is replaced by new value e.g.
      * "myParam"="http://onto.fel.cvut.cz/resources/dea95aca-590e-4e77-8a0c-458814cc82b5
      *
-     * @param parameters         Http query parameters.
-     * @param files              Multipart files referenced by the parameters.
-     * @param newStreamResources List of new stream resources created by this method,
-     *                           empty list should be provided here.
-     *                           // TODO this is parameter should be removed as it is workaround
-     *                           //for issue #XXX
+     * @param parameters Http query parameters.
+     * @param files Multipart files referenced by the parameters.
      * @return New parameters where each found reference is replaced by stream resource url.
      */
-    public MultiValueMap<String, String> resolveResources(
-        MultiValueMap<String, String> parameters,
-        MultipartFile[] files,
-        List<StreamResourceDTO> newStreamResources) {
+    public MultiValueMap<String, String> resolveResources(MultiValueMap<String, String> parameters, MultipartFile[] files) {
         MultiValueMap<String, String> newParameters = new LinkedMultiValueMap<>(parameters);
 
         parameters.entrySet().stream()
@@ -78,7 +70,6 @@ public MultiValueMap<String, String> resolveResources(
                     MultipartFile multipartFile = multipartFileOptional.get();
                     try {
                         StreamResourceDTO res = resourceRegisterHelper.registerStreamResource(multipartFile.getContentType(), multipartFile.getInputStream());
-                        newStreamResources.add(res);
                         newParameters.replace(e.getKey(), Collections.singletonList(res.getPersistentUri()));
                     } catch (IOException ex) {
                         LOG.error(ex.getMessage(), ex);

diff --git a/s-pipes-web/src/main/java/cz/cvut/spipes/rest/util/ResourceRegisterHelper.java b/s-pipes-web/src/main/java/cz/cvut/spipes/rest/util/ResourceRegisterHelper.java
@@ -1,6 +1,5 @@
 package cz.cvut.spipes.rest.util;
 
-import cz.cvut.spipes.registry.StreamResource;
 import cz.cvut.spipes.registry.StreamResourceRegistry;
 import cz.cvut.spipes.rest.StreamResourceDTO;
 import org.apache.commons.io.IOUtils;
@@ -30,9 +29,7 @@ public StreamResourceDTO registerStreamResource(String contentType, InputStream
         final byte[] data;
         try {
             data = IOUtils.toByteArray(body);
-            StreamResource streamResource = StreamResourceRegistry.getInstance()
-                .registerResource(res.getId(), data, contentType);
-            res.attachStreamResource(streamResource);
+            StreamResourceRegistry.getInstance().registerResource(res.getId(), data, contentType);
             LOG.info("Resource content size: {}", data.length);
         } catch (IOException e) {
             LOG.error("Unable to read payload: ", e);

diff --git a/s-pipes-web/src/test/java/cz/cvut/spipes/rest/util/MultipartFileResourceResolverTest.java b/s-pipes-web/src/test/java/cz/cvut/spipes/rest/util/MultipartFileResourceResolverTest.java
@@ -15,7 +15,6 @@
 import org.springframework.web.multipart.MultipartFile;
 
 import java.net.URI;
-import java.util.LinkedList;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -55,7 +54,7 @@ void testResolveResourcesReplacesFileReferencesWithUrisInParameters() {
 
         MultipartFile[] files = new MultipartFile[]{testFile1, testFile2};
 
-        MultiValueMap<String, String> newParameters = resolver.resolveResources(parameters, files, new LinkedList<>());
+        MultiValueMap<String, String> newParameters = resolver.resolveResources(parameters, files);
 
         assertEquals(parameters.keySet(), newParameters.keySet());
         assertEquals(1, newParameters.get("testKey1").size());
@@ -87,7 +86,7 @@ void testResolveResourcesDoesNotReplaceInvalidFileReferences() {
 
         MultipartFile[] files = new MultipartFile[]{testFile1, testFile2};
 
-        MultiValueMap<String, String> newParameters = resolver.resolveResources(parameters, files, new LinkedList<>());
+        MultiValueMap<String, String> newParameters = resolver.resolveResources(parameters, files);
 
         assertEquals(parameters, newParameters);
     }