Add delete and download to ArtifactControl

Download first gets an unprotected, temporary download URL. It then loads an
iframe with the retrieved URL, which causes to browser to download the file.
This is done to prevent a reload of the page while at the same time use the
standard browser file download functionality. Other solutions found cause
problems with bigger files since they download the file first and then generate
a binary blob in the DOM.

Delete just deletes an artifact and reloads the artifact list. A potential
problem could occur if the removal in the archive fails while the file is
already deleted from the database.

Signed-off-by: Armin Friedl <dev@friedl.net>

service/fling/src/main/java/net/friedl/fling/controller/ArtifactController.java

@@ -5,8 +5,12 @@ import java.util.List;
 import javax.servlet.http.HttpServletRequest;
 
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.core.io.InputStreamResource;
+import org.springframework.core.io.Resource;
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.DeleteMapping;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PatchMapping;
 import org.springframework.web.bind.annotation.PathVariable;
@@ -17,6 +21,7 @@ import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RestController;
 
 import net.friedl.fling.model.dto.ArtifactDto;
+import net.friedl.fling.persistence.archive.ArchiveException;
 import net.friedl.fling.service.ArtifactService;
 
 @RestController
@@ -30,12 +35,12 @@ public class ArtifactController {
         this.artifactService = artifactService;
     }
 
-    @GetMapping(path = "/artifacts", params="flingId")
+    @GetMapping(path = "/artifacts", params = "flingId")
     public List<ArtifactDto> getArtifacts(@RequestParam Long flingId) {
         return artifactService.findAllArtifacts(flingId);
     }
 
-    @GetMapping(path = "/artifacts", params="artifactId")
+    @GetMapping(path = "/artifacts", params = "artifactId")
     public ResponseEntity<ArtifactDto> getArtifact(@RequestParam Long artifactId) {
         return ResponseEntity.of(artifactService.findArtifact(artifactId));
     }
@@ -46,7 +51,32 @@ public class ArtifactController {
     }
 
     @PatchMapping(path = "/artifacts/{artifactId}", consumes = MediaType.APPLICATION_JSON_VALUE)
-    public ArtifactDto patchArtifactDto(@PathVariable Long artifactId, @RequestBody String body) {
+    public ArtifactDto patchArtifact(@PathVariable Long artifactId, @RequestBody String body) {
         return artifactService.mergeArtifact(artifactId, body);
     }
+
+    @DeleteMapping(path = "/artifacts/{artifactId}")
+    public void deleteArtifact(@PathVariable Long artifactId) throws ArchiveException {
+        artifactService.deleteArtifact(artifactId);
+    }
+
+    @GetMapping(path = "/artifacts/{artifactId}/downloadid")
+    public String getDownloadId(@PathVariable Long artifactId) {
+        return artifactService.generateDownloadId(artifactId);
+    }
+
+    @GetMapping(path = "/artifacts/{artifactId}/{downloadId}/download")
+    public ResponseEntity<Resource> downloadArtifact(@PathVariable Long artifactId, @PathVariable String downloadId)
+            throws ArchiveException {
+
+        var artifact = artifactService.findArtifact(artifactId).orElseThrow();
+        var stream = new InputStreamResource(artifactService.downloadArtifact(downloadId));
+
+        return ResponseEntity.ok()
+                .header(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename=\"" + artifact.getName() + "\"")
+                .contentLength(artifact.getSize())
+                .contentType(MediaType.APPLICATION_OCTET_STREAM)
+                .body(stream);
+    }
+
 }

service/fling/src/main/java/net/friedl/fling/model/mapper/ArtifactMapper.java

@@ -3,6 +3,7 @@ package net.friedl.fling.model.mapper;
 import java.lang.reflect.Field;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 
 import org.mapstruct.Mapper;
 
@@ -19,6 +20,10 @@ public abstract class ArtifactMapper {
 
     public abstract List<ArtifactDto> map(List<ArtifactEntity> artifactEntities);
 
+    public Optional<ArtifactDto> map(Optional<ArtifactEntity> artifactEntity) {
+        return artifactEntity.map(a -> map(a));
+    }
+
     public ArtifactDto merge(ArtifactDto originalArtifactDto, Map<String, Object> patch) {
         ArtifactDto mergedArtifactDto = new ArtifactDto();
 

service/fling/src/main/java/net/friedl/fling/persistence/archive/Archive.java

@@ -32,4 +32,11 @@ public interface Archive {
             throw new ArchiveException(ex);
         }
     }
+
+    /**
+     * Delete an artifact
+     *
+     * @param id The unique artifact id as returned by {@link Archive#store}
+     */
+    void remove(String id) throws ArchiveException;
 }

service/fling/src/main/java/net/friedl/fling/persistence/archive/impl/FileSystemArchive.java

@@ -6,6 +6,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
+import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.nio.file.StandardOpenOption;
 import java.security.MessageDigest;
@@ -34,8 +35,7 @@ public class FileSystemArchive implements Archive {
             var path = Paths.get(configuration.getDirectory(), id);
             FileInputStream fis = new FileInputStream(path.toFile());
             return fis;
-        }
+        } catch (FileNotFoundException ex) {
-        catch (FileNotFoundException ex) {
             throw new ArchiveException(ex);
         }
     }
@@ -56,12 +56,21 @@ public class FileSystemArchive implements Archive {
             fc.close();
             return fileStoreId;
 
-        }
+        } catch (IOException ex) {
-        catch (IOException ex) {
             throw new ArchiveException(ex);
         }
     }
 
+    @Override
+    public void remove(String id) throws ArchiveException {
+        var path = Paths.get(configuration.getDirectory(), id);
+        try {
+            Files.deleteIfExists(path);
+        } catch (IOException e) {
+            throw new ArchiveException("Could not delete file at " + path.toString(), e);
+        }
+    }
+
     private String hexEncode(byte[] fileStoreId) {
         StringBuilder sb = new StringBuilder(fileStoreId.length * 2);
         for (byte b : fileStoreId)

service/fling/src/main/java/net/friedl/fling/security/FlingWebSecurityConfigurer.java

@@ -57,6 +57,10 @@ public class FlingWebSecurityConfigurer extends WebSecurityConfigurerAdapter {
             .antMatchers("/api/auth/**")
             .permitAll()
         .and()
+        .authorizeRequests()
+            .antMatchers("/api/artifacts/{artifactId}/{downloadId}/download")
+            .permitAll()
+        .and()
         .authorizeRequests()
             .antMatchers("/api/**")
             .hasAuthority(FlingAuthority.FLING_OWNER.name())

service/fling/src/main/java/net/friedl/fling/service/ArtifactService.java

@@ -56,7 +56,7 @@ public class ArtifactService {
     }
 
     public Optional<ArtifactDto> findArtifact(Long artifactId) {
-        return null;
+        return artifactMapper.map(artifactRepository.findById(artifactId));
     }
 
     public ArtifactDto mergeArtifact(Long artifactId, String body) {
@@ -74,4 +74,19 @@ public class ArtifactService {
 
         return artifactMapper.map(artifactRepository.getOne(artifactId));
     }
+
+    public void deleteArtifact(Long artifactId) throws ArchiveException {
+        var doi = artifactRepository.getOne(artifactId).getDoi();
+        artifactRepository.deleteById(artifactId);
+        archive.remove(doi);
+    }
+
+    public String generateDownloadId(Long artifactId) {
+        // TODO: This id is not secured! Generate temporary download id
+        return artifactRepository.getOne(artifactId).getDoi();
+    }
+
+    public InputStream downloadArtifact(String downloadId) throws ArchiveException {
+        return archive.get(downloadId);
+    }
 }

web/fling/src/components/admin/FlingArtifacts.jsx

@@ -1,16 +1,38 @@
 import log from 'loglevel';
 import React, {useState, useEffect, useRef} from 'react';
+import {useHistory, useLocation} from 'react-router-dom';
 
 import classNames from 'classnames';
 
 import {artifactClient} from '../../util/flingclient';
 
 function FlingArtifactControl(props) {
+    let history = useHistory();
+    let iframeContainer = useRef(null);
+
+    function handleDelete(ev) {
+        artifactClient.deleteArtifact(props.artifact.id)
+            .then(() => props.reloadArtifactsFn());
+    }
+
+    function handleDownload(ev) {
+        artifactClient.downloadArtifact(props.artifact.id)
+            .then(url => {
+                // We need this iframe hack because with a regular href, while
+                // the browser downloads the file fine, it also reloads the page, hence
+                // loosing all logs and state
+                let frame = document.createElement("iframe");
+                frame.src = url;
+                iframeContainer.current.appendChild(frame);
+            });
+    }
+
     return(
         <div className={`btn-group ${props.hidden ? "d-invisible": "d-visible"}`}>
-          <button className="btn btn-sm"><i className="icon icon-delete"/></button>
+          <button className="btn btn-sm" onClick={handleDelete}><i className="icon icon-delete"/></button>
           <button className="btn btn-sm"><i className="icon icon-edit"/></button>
-          <button className="btn btn-sm"><i className="icon icon-download"/></button>
+          <button className="btn btn-sm" onClick={handleDownload}><i className="icon icon-download"/></button>
+          <div className="d-hide" ref={iframeContainer}/>
         </div>
     );
 }
@@ -23,7 +45,7 @@ function FlingArtifactRow(props) {
           <td>{props.artifact.name}</td>
           <td>{props.artifact.version}</td>
           <td/>
-          <td><FlingArtifactControl hidden={!hovered} /></td>
+          <td><FlingArtifactControl artifact={props.artifact} reloadArtifactsFn={props.reloadArtifactsFn} hidden={!hovered} /></td>
         </tr>
     );
 }
@@ -61,7 +83,7 @@ export default function FlingArtifacts(props) {
             .then(result => {
                 log.debug(`Got ${result.length} artifacts`);
                 for(let artifact of result) {
-                    artifacts.push(<FlingArtifactRow key={artifact.id} artifact={artifact} />);
+                    artifacts.push(<FlingArtifactRow key={artifact.id} artifact={artifact} reloadArtifactsFn={getArtifacts} />);
                 }
 
                 setArtifacts(artifacts);

web/fling/src/style/fling.scss

@@ -90,6 +90,12 @@ body {
 \*************/
 .panel {
     background-color: #ffffff;
+    border: none;
+    @include shadow;
+
+    .panel-body {
+        overflow-y: visible;
+    }
 }
 
 /*************\