UFAL/Improve file preview generating (#972)

Paurikova2 · milanmajchrak · web-flow · commit 7c920b24fd22 · 2025-06-13T10:46:27.000+02:00
* get name and size from metadata and header of file, avoid input stream using

* remove temp file, checkstyle, do not load full file

* add { } after if

* added check for max preview file

* used ZipFile and TarArchived for filepreview generating

* added removed lines

* used 7z for zip and tar files

* removed 7z and used zip and tar entry

* improved file previrew generating speed, used string builder, xml builder, authorization only if is required

* checkstyle, return boolean from haspreview and previrews from getPreview, replaced return with continue

* fix problem with hibernate session

* fix .tar.gz generating

* skip fully entry for tar

* added indexes for speed up queries

* added license header

* named constant by upper case

* inicialized fileInfo, refactorization of code based on copilot review

---------

Co-authored-by: milanmajchrak &lt;90026355+milanmajchrak@users.noreply.github.com&gt;
diff --git a/dspace-api/src/main/java/org/dspace/content/BitstreamServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/BitstreamServiceImpl.java
@@ -7,6 +7,7 @@
  */
 package org.dspace.content;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.sql.SQLException;
@@ -315,6 +316,15 @@ public InputStream retrieve(Context context, Bitstream bitstream)
         return bitstreamStorageService.retrieve(context, bitstream);
     }
 
+    @Override
+    public File retrieveFile(Context context, Bitstream bitstream, boolean authorization)
+            throws IOException, SQLException, AuthorizeException {
+        if (authorization) {
+            authorizeService.authorizeAction(context, bitstream, Constants.READ);
+        }
+        return bitstreamStorageService.retrieveFile(context, bitstream);
+    }
+
     @Override
     public boolean isRegisteredBitstream(Bitstream bitstream) {
         return bitstreamStorageService.isRegisteredBitstream(bitstream.getInternalId());
diff --git a/dspace-api/src/main/java/org/dspace/content/PreviewContentServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/PreviewContentServiceImpl.java
diff --git a/dspace-api/src/main/java/org/dspace/content/dao/PreviewContentDAO.java b/dspace-api/src/main/java/org/dspace/content/dao/PreviewContentDAO.java
@@ -33,6 +33,16 @@ public interface PreviewContentDAO extends GenericDAO<PreviewContent> {
      */
     List<PreviewContent> findByBitstream(Context context, UUID bitstreamId) throws SQLException;
 
+    /**
+     *  Returns true if the bitstream has associated preview content.
+     *
+     * @param context       DSpace context
+     * @param bitstream     The bitstream to get bitstream UUID
+     * @return              True if preview content exists, false otherwise
+     * @throws SQLException If a database error occurs
+     */
+    boolean hasPreview(Context context, Bitstream bitstream) throws SQLException;
+
     /**
      *  Find all preview content based on bitstream that are the root directory.
      *
@@ -41,5 +51,5 @@ public interface PreviewContentDAO extends GenericDAO<PreviewContent> {
      * @return              List of found preview content
      * @throws SQLException If a database error occurs
      */
-    List<PreviewContent> hasPreview(Context context, Bitstream bitstream) throws SQLException;
+    List<PreviewContent> getPreview(Context context, Bitstream bitstream) throws SQLException;
 }
diff --git a/dspace-api/src/main/java/org/dspace/content/dao/impl/PreviewContentDAOImpl.java b/dspace-api/src/main/java/org/dspace/content/dao/impl/PreviewContentDAOImpl.java
@@ -40,7 +40,16 @@ public List<PreviewContent> findByBitstream(Context context, UUID bitstreamId) t
     }
 
     @Override
-    public List<PreviewContent> hasPreview(Context context, Bitstream bitstream) throws SQLException {
+    public boolean hasPreview(Context context, Bitstream bitstream) throws SQLException {
+        Query query = createQuery(context,
+                "SELECT COUNT(pc) FROM " + PreviewContent.class.getSimpleName() +
+                        " pc WHERE pc.bitstream.id = :bitstream_id");
+        query.setParameter("bitstream_id", bitstream.getID());
+        return count(query) > 0;
+    }
+
+    @Override
+    public List<PreviewContent> getPreview(Context context, Bitstream bitstream) throws SQLException {
         // select only data from the previewcontent table whose ID is not a child in the preview2preview table
         Query query = getHibernateSession(context).createNativeQuery(
                 "SELECT pc.* FROM previewcontent pc " +
diff --git a/dspace-api/src/main/java/org/dspace/content/service/BitstreamService.java b/dspace-api/src/main/java/org/dspace/content/service/BitstreamService.java
@@ -7,6 +7,7 @@
  */
 package org.dspace.content.service;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.sql.SQLException;
@@ -154,7 +155,7 @@ public Bitstream register(Context context, int assetstore, String bitstreamPath)
     public void setFormat(Context context, Bitstream bitstream, BitstreamFormat bitstreamFormat) throws SQLException;
 
     /**
-     * Retrieve the contents of the bitstream
+     * Retrieve the contents of the bitstream.
      *
      * @param context   DSpace context object
      * @param bitstream DSpace bitstream
@@ -166,6 +167,20 @@ public Bitstream register(Context context, int assetstore, String bitstreamPath)
     public InputStream retrieve(Context context, Bitstream bitstream)
         throws IOException, SQLException, AuthorizeException;
 
+    /**
+     * Retrieve the contents of the bitstream.
+     *
+     * @param context   DSpace context object
+     * @param bitstream DSpace bitstream
+     * @param authorization true if authorization is required else false
+     * @return a File from which the bitstream can be read.
+     * @throws IOException        if IO error
+     * @throws SQLException       if database error
+     * @throws AuthorizeException if authorization error
+     */
+    public File retrieveFile(Context context, Bitstream bitstream, boolean authorization)
+            throws IOException, SQLException, AuthorizeException;
+
     /**
      * Determine if this bitstream is registered (available elsewhere on
      * filesystem than in assetstore). More about registered items:
diff --git a/dspace-api/src/main/java/org/dspace/content/service/PreviewContentService.java b/dspace-api/src/main/java/org/dspace/content/service/PreviewContentService.java
@@ -7,7 +7,7 @@
  */
 package org.dspace.content.service;
 
-import java.io.InputStream;
+import java.io.File;
 import java.sql.SQLException;
 import java.util.List;
 import java.util.Map;
@@ -82,14 +82,25 @@ PreviewContent create(Context context, Bitstream bitstream, String name, String
      */
     List<PreviewContent> findByBitstream(Context context, UUID bitstream_id) throws SQLException;
 
+    /**
+     * Returns true if the bitstream has associated preview content.
+     *
+     * @param context        DSpace context
+     * @param bitstream      The bitstream to get bitstream UUID
+     * @return               True if preview content exists, false otherwise
+     * @throws SQLException  If a database error occurs
+     */
+    boolean hasPreview(Context context, Bitstream bitstream) throws SQLException;
+
     /**
      * Find all preview content based on bitstream that are the root directory.
      *
      * @param context        DSpace context
      * @param bitstream      The bitstream to get bitstream UUID
+     * @return               List of preview contents
      * @throws SQLException  If a database error occurs
      */
-    List<PreviewContent> hasPreview(Context context, Bitstream bitstream) throws SQLException;
+    List<PreviewContent> getPreview(Context context, Bitstream bitstream) throws SQLException;
 
     /**
      * Find all preview contents from database.
@@ -104,9 +115,11 @@ PreviewContent create(Context context, Bitstream bitstream, String name, String
      *
      * @param context DSpace context object
      * @param bitstream check if this bitstream could be previewed
+     * @param authorization true if authorization is required else false
      * @return true if the bitstream could be previewed, false otherwise
      */
-    boolean canPreview(Context context, Bitstream bitstream) throws SQLException, AuthorizeException;
+    boolean canPreview(Context context, Bitstream bitstream, boolean authorization)
+            throws SQLException, AuthorizeException;
 
     /**
      * Return converted ZIP file content into FileInfo classes.
@@ -143,13 +156,13 @@ PreviewContent create(Context context, Bitstream bitstream, String name, String
     FileInfo createFileInfo(PreviewContent pc);
 
     /**
-     * Convert InputStream of the ZIP file into FileInfo classes.
+     * Convert File of the ZIP file into FileInfo classes.
      *
      * @param context DSpace context object
      * @param bitstream previewing bitstream
-     * @param inputStream content of the zip file
+     * @param file content of the zip file
      * @return List of FileInfo classes where is wrapped ZIP file content
      */
-    List<FileInfo> processInputStreamToFilePreview(Context context, Bitstream bitstream, InputStream inputStream)
+    List<FileInfo> processFileToFilePreview(Context context, Bitstream bitstream, File file)
             throws Exception;
 }
diff --git a/dspace-api/src/main/java/org/dspace/scripts/filepreview/FilePreview.java b/dspace-api/src/main/java/org/dspace/scripts/filepreview/FilePreview.java
@@ -22,7 +22,6 @@
 import org.dspace.content.Bitstream;
 import org.dspace.content.Bundle;
 import org.dspace.content.Item;
-import org.dspace.content.PreviewContent;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.ItemService;
 import org.dspace.content.service.PreviewContentService;
@@ -147,21 +146,20 @@ private void generateItemFilePreviews(Context context, UUID itemUUID) throws Exc
         for (Bundle bundle : bundles) {
             List<Bitstream> bitstreams = bundle.getBitstreams();
             for (Bitstream bitstream : bitstreams) {
-                boolean canPreview = previewContentService.canPreview(context, bitstream);
+                boolean canPreview = previewContentService.canPreview(context, bitstream, false);
                 if (!canPreview) {
-                    return;
+                    continue;
                 }
-                List<PreviewContent> prContents = previewContentService.hasPreview(context, bitstream);
                 // Generate new content if we didn't find any
-                if (!prContents.isEmpty()) {
-                    return;
+                if (previewContentService.hasPreview(context, bitstream)) {
+                    continue;
                 }
 
                 List<FileInfo> fileInfos = previewContentService.getFilePreviewContent(context, bitstream);
                 // Do not store HTML content in the database because it could be longer than the limit
                 // of the database column
                 if (StringUtils.equals("text/html", bitstream.getFormat(context).getMIMEType())) {
-                    return;
+                    continue;
                 }
 
                 for (FileInfo fi : fileInfos) {
diff --git a/dspace-api/src/main/java/org/dspace/storage/bitstore/BitStoreService.java b/dspace-api/src/main/java/org/dspace/storage/bitstore/BitStoreService.java
@@ -7,6 +7,7 @@
  */
 package org.dspace.storage.bitstore;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.List;
@@ -45,6 +46,16 @@ public interface BitStoreService {
      */
     public InputStream get(Bitstream bitstream) throws IOException;
 
+    /**
+     * Retrieve the bits for bitstream
+     *
+     * @param bitstream DSpace Bitstream object
+     * @return The File
+     * @throws java.io.IOException If a problem occurs while retrieving the bits, or if no
+     *                             asset with ID exists in the store
+     */
+    public File getFile(Bitstream bitstream) throws IOException;
+
     /**
      * Store a stream of bits.
      *
diff --git a/dspace-api/src/main/java/org/dspace/storage/bitstore/BitstreamStorageServiceImpl.java b/dspace-api/src/main/java/org/dspace/storage/bitstore/BitstreamStorageServiceImpl.java
@@ -7,6 +7,7 @@
  */
 package org.dspace.storage.bitstore;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.sql.SQLException;
@@ -214,6 +215,13 @@ public InputStream retrieve(Context context, Bitstream bitstream)
         return this.getStore(storeNumber).get(bitstream);
     }
 
+    @Override
+    public File retrieveFile(Context context, Bitstream bitstream)
+        throws IOException {
+        Integer storeNumber = bitstream.getStoreNumber();
+        return this.getStore(storeNumber).getFile(bitstream);
+    }
+
     @Override
     public void cleanup(boolean deleteDbRecords, boolean verbose) throws SQLException, IOException, AuthorizeException {
         Context context = new Context(Context.Mode.BATCH_EDIT);
diff --git a/dspace-api/src/main/java/org/dspace/storage/bitstore/DSBitStoreService.java b/dspace-api/src/main/java/org/dspace/storage/bitstore/DSBitStoreService.java
@@ -208,7 +208,7 @@ private synchronized static void deleteParents(File file) {
      * @return The corresponding file in the file system, or <code>null</code>
      * @throws IOException If a problem occurs while determining the file
      */
-    protected File getFile(Bitstream bitstream) throws IOException {
+    public File getFile(Bitstream bitstream) throws IOException {
         // Check that bitstream is not null
         if (bitstream == null) {
             return null;
diff --git a/dspace-api/src/main/java/org/dspace/storage/bitstore/S3BitStoreService.java b/dspace-api/src/main/java/org/dspace/storage/bitstore/S3BitStoreService.java
@@ -305,6 +305,29 @@ public InputStream get(Bitstream bitstream) throws IOException {
         }
     }
 
+    @Override
+    public File getFile(Bitstream bitstream) throws IOException {
+        String key = getFullKey(bitstream.getInternalId());
+        // Strip -R from bitstream key if it's registered
+        if (isRegisteredBitstream(key)) {
+            key = key.substring(REGISTERED_FLAG.length());
+        }
+        try {
+            File tempFile = File.createTempFile("s3-disk-copy-" + UUID.randomUUID(), "temp");
+            tempFile.deleteOnExit();
+
+            GetObjectRequest getObjectRequest = new GetObjectRequest(bucketName, key);
+
+            Download download = tm.download(getObjectRequest, tempFile);
+            download.waitForCompletion();
+
+            return tempFile;
+        } catch (AmazonClientException | InterruptedException e) {
+            log.error("getFile(" + key + ")", e);
+            throw new IOException(e);
+        }
+    }
+
     /**
      * Store a stream of bits.
      *
diff --git a/dspace-api/src/main/java/org/dspace/storage/bitstore/service/BitstreamStorageService.java b/dspace-api/src/main/java/org/dspace/storage/bitstore/service/BitstreamStorageService.java
@@ -7,6 +7,7 @@
  */
 package org.dspace.storage.bitstore.service;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.sql.SQLException;
@@ -126,6 +127,18 @@ public UUID register(Context context, Bitstream bitstream, int assetstore, Strin
     public InputStream retrieve(Context context, Bitstream bitstream)
         throws SQLException, IOException;
 
+    /**
+     * Retrieve the file of the bitstream with ID. If the bitstream does not
+     * exist, or is marked deleted, returns null.
+     *
+     * @param context   The current context
+     * @param bitstream The bitstream to retrieve
+     * @return The file, or null
+     * @throws IOException  If a problem occurs while retrieving the bits
+     * @throws SQLException If a problem occurs accessing the RDBMS
+     */
+    public File retrieveFile(Context context, Bitstream bitstream) throws SQLException, IOException;
+
     /**
      * Clean up the bitstream storage area. This method deletes any bitstreams
      * which are more than 1 hour old and marked deleted. The deletions cannot
diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2025.06.09__Added_Indexes_To_Preview_Tables.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2025.06.09__Added_Indexes_To_Preview_Tables.sql
@@ -0,0 +1,25 @@
+--
+-- The contents of this file are subject to the license and copyright
+-- detailed in the LICENSE and NOTICE files at the root of the source
+-- tree and available online at
+--
+-- http://www.dspace.org/license/
+--
+
+-- ===================================================================
+-- PERFORMANCE INDEXES
+-- ===================================================================
+
+--
+-- Index to speed up queries filtering previewcontent by bitstream_id,
+-- used in hasPreview() and getPreview() JOIN with bitstream table.
+--
+CREATE INDEX idx_previewcontent_bitstream_id
+ON previewcontent (bitstream_id);
+
+--
+-- Index to optimize NOT EXISTS subquery in getPreview(),
+-- checking for existence of child_id in preview2preview.
+--
+CREATE INDEX idx_preview2preview_child_id
+ON preview2preview (child_id);
diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V7.6_2025.06.09__Added_Indexes_To_Preview_Tables.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V7.6_2025.06.09__Added_Indexes_To_Preview_Tables.sql
@@ -0,0 +1,25 @@
+--
+-- The contents of this file are subject to the license and copyright
+-- detailed in the LICENSE and NOTICE files at the root of the source
+-- tree and available online at
+--
+-- http://www.dspace.org/license/
+--
+
+-- ===================================================================
+-- PERFORMANCE INDEXES
+-- ===================================================================
+
+--
+-- Index to speed up queries filtering previewcontent by bitstream_id,
+-- used in hasPreview() and getPreview() JOIN with bitstream table.
+--
+CREATE INDEX idx_previewcontent_bitstream_id
+ON previewcontent (bitstream_id);
+
+--
+-- Index to optimize NOT EXISTS subquery in getPreview(),
+-- checking for existence of child_id in preview2preview.
+--
+CREATE INDEX idx_preview2preview_child_id
+ON preview2preview (child_id);
diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/MetadataBitstreamRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/MetadataBitstreamRestRepository.java
@@ -107,15 +107,14 @@ public Page<MetadataBitstreamWrapperRest> findByHandle(@Parameter(value = "handl
             for (Bitstream bitstream : bitstreams) {
                 String url = previewContentService.composePreviewURL(context, item, bitstream, contextPath);
                 List<FileInfo> fileInfos = new ArrayList<>();
-                boolean canPreview = previewContentService.canPreview(context, bitstream);
+                boolean canPreview = previewContentService.canPreview(context, bitstream, false);
                 String mimeType = bitstream.getFormat(context).getMIMEType();
                 // HTML content could be longer than the limit, so we do not store it in the DB.
                 // It has to be generated even if property is false.
                 if (StringUtils.equals(mimeType, TEXT_HTML_MIME_TYPE) || canPreview) {
                     try {
-                        List<PreviewContent> prContents = previewContentService.hasPreview(context, bitstream);
                         // Generate new content if we didn't find any
-                        if (prContents.isEmpty()) {
+                        if (!previewContentService.hasPreview(context, bitstream)) {
                             boolean allowComposePreviewContent = configurationService.getBooleanProperty
                                     ("create.file-preview.on-item-page-load", false);
                             if (allowComposePreviewContent) {
@@ -131,6 +130,7 @@ public Page<MetadataBitstreamWrapperRest> findByHandle(@Parameter(value = "handl
                                 }
                             }
                         } else {
+                            List<PreviewContent> prContents = previewContentService.getPreview(context, bitstream);
                             for (PreviewContent pc : prContents) {
                                 fileInfos.add(previewContentService.createFileInfo(pc));
                             }
diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/PreviewContentServiceImplIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/PreviewContentServiceImplIT.java
