SEBSP-200 fixed multiple entries in screenshot_data table

Author: anhefti

src/main/java/ch/ethz/seb/sps/server/servicelayer/impl/ScreenshotStore_FullRDBMS.java

@@ -207,7 +207,12 @@ private void applyBatchStore(final Collection<ScreenshotQueueData> batch) {
                     .executeWithoutResult(status -> {
 
                         // store all screenshot data in batch and grab generated keys put back to records
-                        batch.forEach(data -> this.screenshotDataRecordMapper.insert(data.record));
+                        batch.forEach(data -> {
+                            if (data.record.getId() == null) {
+                                // store only screenshot data that has not been stored before
+                                this.screenshotDataRecordMapper.insert(data.record);
+                            }
+                        });
                         this.sqlSessionTemplate.flushStatements();
 
                         // now store all screenshots within respective generated ids in batch
@@ -219,19 +224,19 @@ private void applyBatchStore(final Collection<ScreenshotQueueData> batch) {
                         this.sqlSessionTemplate.flushStatements();
                     });
         } catch (final TransactionException te) {
-            log.error(
-                    "Failed to batch store screenshot data. Transaction has failed... put data back to queue. Cause: ",
-                    te);
-            this.screenshotDataQueue.addAll(batch);
+            putBatchBackToQueue(batch, te);
         } catch (final RuntimeException re) {
             if (re instanceof DataIntegrityViolationException) {
                 // try to store in sequence to filter out the corrupted data and store the valid data
                 batch.forEach(data -> {
                     try {
                         this.transactionTemplate
                                 .executeWithoutResult(status -> {
-                                    this.screenshotDataRecordMapper.insert(data.record);
-                                    this.sqlSessionTemplate.flushStatements();
+                                    // store only screenshot data that has not been stored before
+                                    if (data.record.getId() == null) {
+                                        this.screenshotDataRecordMapper.insert(data.record);
+                                        this.sqlSessionTemplate.flushStatements();
+                                    }
 
                                     this.screenshotMapper.insert(new BlobContent(
                                             data.record.getId(),
@@ -246,14 +251,17 @@ private void applyBatchStore(final Collection<ScreenshotQueueData> batch) {
                     }
                 });
             } else {
-                log.error("Failed to batch store screenshot data. Transaction has failed... put data back to queue. Cause: ", re);
-                if (Utils.enoughHeapMemLeft(1000)) {
-                    this.screenshotDataQueue.addAll(batch);
-                } else {
-                    log.warn("There is not enough heap memory left to store the screenshots that failed to store. {} Screenshots are skipped now", batch.size());
-                }
+                putBatchBackToQueue(batch, re);
             }
         }
     }
 
+    private void putBatchBackToQueue(Collection<ScreenshotQueueData> batch, RuntimeException re) {
+        log.error("Failed to batch store screenshot data. Transaction has failed... put data back to queue. Cause: {}", re.getMessage());
+        if (Utils.enoughHeapMemLeft(1000)) {
+            this.screenshotDataQueue.addAll(batch);
+        } else {
+            log.warn("There is not enough heap memory left to store the screenshots that failed to store. {} Screenshots are skipped now", batch.size());
+        }
+    }
 }

src/main/java/ch/ethz/seb/sps/server/servicelayer/impl/ScreenshotStore_S3.java

@@ -167,9 +167,12 @@ private void applySingleStore(final Collection<ScreenshotQueueData> batch) {
         // probably less performant but in case S3 do not support batch store
         batch.forEach(data -> {
             try {
-                // store screenshot data of single screenshot within DB
-                screenshotDataRecordMapper.insert(data.record);
-                this.sqlSessionTemplate.flushStatements();
+                
+                // store screenshot data of single screenshot within DB only if it is not already in the DB
+                if (data.record.getId() == null) {
+                    screenshotDataRecordMapper.insert(data.record);
+                    this.sqlSessionTemplate.flushStatements();
+                }
 
                 // store single screenshot picture to S3
                 this.s3DAO.uploadItem(
@@ -180,7 +183,11 @@ private void applySingleStore(final Collection<ScreenshotQueueData> batch) {
 
             } catch (Exception e) {
                 log.error("Failed to single store screenshot data... put data back to queue. Cause: {}", e.getMessage());
-                this.screenshotDataQueue.add(data);
+                if (Utils.enoughHeapMemLeft(1000)) {
+                    this.screenshotDataQueue.add(data);
+                } else {
+                    log.warn("There is not enough heap memory left to store the screenshots that failed to store. 1 Screenshot is skipped now");
+                }
             }
         });
     }
@@ -189,7 +196,12 @@ private void applyBatchStore(final Collection<ScreenshotQueueData> batch) {
         try {
             this.transactionTemplate.executeWithoutResult(status -> {
                 // store all screenshot data in batch and grab generated keys put back to records
-                batch.forEach(data -> this.screenshotDataRecordMapper.insert(data.record));
+                batch.forEach(data -> {
+                    if (data.record.getId() == null) {
+                        // store only screenshot data that has not been stored before
+                        this.screenshotDataRecordMapper.insert(data.record);
+                    }
+                });
                 this.sqlSessionTemplate.flushStatements();
 
                 // now store all screenshots within respective generated ids in batch
@@ -206,15 +218,12 @@ private void applyBatchStore(final Collection<ScreenshotQueueData> batch) {
                 });
 
                 //upload batch to s3 store
-                this.s3DAO.uploadItemBatch(batchItems).onError(e -> {
-                    log.error("Failed to upload batch to S3 service. Transaction has failed... put data back to queue. Cause: ", e);
-                    this.screenshotDataQueue.addAll(batch);
-                });
+                this.s3DAO.uploadItemBatch(batchItems).getOrThrow();
 
             });
 
         } catch (final TransactionException te) {
-            log.error("Failed to batch store screenshot data. Transaction has failed... put data back to queue. Cause: ", te);
+            log.error("Failed to batch store screenshot data. Transaction has failed... put data back to queue. Cause: {}", te.getMessage());
             if (Utils.enoughHeapMemLeft(1000)) {
                 this.screenshotDataQueue.addAll(batch);
             } else {

src/test/java/ch/ethz/seb/sps/integrationtests/datalayer/batis/dao/ScreenshotS3FailTest.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2025 ETH Zürich, IT Services
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+package ch.ethz.seb.sps.integrationtests.datalayer.batis.dao;
+
+import static org.junit.Assert.*;
+
+import ch.ethz.seb.sps.integrationtests.ServiceTest_FULL_RDBMS;
+import ch.ethz.seb.sps.server.datalayer.batis.mapper.ScreenshotDataRecordMapper;
+import ch.ethz.seb.sps.server.datalayer.batis.model.ScreenshotDataRecord;
+import ch.ethz.seb.sps.server.datalayer.dao.impl.S3DAO;
+import ch.ethz.seb.sps.server.servicelayer.impl.ScreenshotStore_S3;
+import org.apache.ibatis.session.ExecutorType;
+import org.apache.ibatis.session.SqlSessionFactory;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mybatis.spring.SqlSessionTemplate;
+import org.springframework.beans.factory.annotation.Autowired;
+
+public class ScreenshotS3FailTest extends ServiceTest_FULL_RDBMS {
+    
+//    @Autowired
+//    private ScreenshotStore_S3 screenshotStore_S3;
+    
+//    @Mock
+//    private S3DAO s3DAO;
+
+    @Autowired
+    private SqlSessionFactory sqlSessionFactory;
+    @Autowired
+    private ScreenshotDataRecordMapper screenshotDataRecordMapper;
+
+    @Test
+    public void testNoDuplicateScreenshotDataStoresOnS3Fail() {
+
+        SqlSessionTemplate sqlSessionTemplate = new SqlSessionTemplate(this.sqlSessionFactory, ExecutorType.BATCH);
+
+        ScreenshotDataRecord screenshotDataRecord = new ScreenshotDataRecord(null, "sessionId", 1738670400000L, 0, "metadata");
+
+        assertNull(screenshotDataRecord.getId());
+
+        screenshotDataRecordMapper.insert(screenshotDataRecord);
+        sqlSessionTemplate.flushStatements();
+
+        assertNotNull(screenshotDataRecord.getId());
+
+        if (screenshotDataRecord.getId() == null) {
+            screenshotDataRecordMapper.insert(screenshotDataRecord);
+            sqlSessionTemplate.flushStatements();
+        }
+        
+        Long num = screenshotDataRecordMapper.countByExample().build().execute();
+        
+        assertTrue(1 == num);
+
+    }
+}