Delete fling, fix race conditions

examples/querysheet.http

@@ -4,6 +4,7 @@
 
 # Authenticate as user
 POST http://localhost:8080/api/auth/user
+Content-Type: application/json
 {"shareId": "shareId", "authCode":"secret"}
 -> jq-set-var :token .
 
@@ -15,9 +16,30 @@ Content-Type: application/json
 {"adminName": "admin", "adminPassword":"123"}
 -> run-hook (restclient-set-var ":token" (buffer-substring-no-properties 1 (line-end-position)))
 
+:token = Authorization: Bearer eyJhbGciOiJIUzI1NiJ9.eyJpYXQiOjE1OTUxNzY4OTksImV4cCI6MTU5NTM1Njg5OSwic3ViIjoiYWRtaW4ifQ.uRh_xBCrBiLQEBah9I8bYWM-Zph-V_pzQVdaGSU5Mlc
+
 # Get all flings
 GET http://localhost:8080/api/fling
-Authorization: Bearer :token
+Content-Type: application/json
+:token
+
+# Add a new fling
+POST http://localhost:8080/api/fling
+Content-Type: application/json
+:token
+{"name": "Unshared Fling from querysheet", "expirationClicks": 12, "shared": false}
+
+# Add a new fling
+POST http://localhost:8080/api/fling
+Content-Type: application/json
+:token
+{"name": "Fling from querysheet with Auth", "expirationClicks": 12, "shared": true, "authCode": "abc"}
+
+# Add a new fling
+POST http://localhost:8080/api/fling
+Content-Type: application/json
+:token
+{"name": "Fling from querysheet with Auth and very long name", "expirationClicks": 12, "shared": true, "authCode": "abc"}
 
 #
 :flingId = dfc208a3-5924-43b4-aa6a-c263541dca5e

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

@@ -129,7 +129,11 @@ public class FileSystemArchive implements ArchiveService {
 
       Path zipDiskPath = archivePath.resolve(flingId.toString() + ".zip");
       log.debug("Deleting fling [.id={}] at {}", flingId, zipDiskPath);
+      if(Files.exists(zipDiskPath)) {
         Files.delete(zipDiskPath);
+      } else {
+        log.warn("No fling disk found at {}", zipDiskPath);
+      }
 
       artifactRepository.findAllByFlingId(flingId).forEach(ar -> ar.setArchived(false));
     }

service/fling/src/test/java/net/friedl/fling/service/archive/FileSystemArchiveTest.java

@@ -4,6 +4,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.Matchers.not;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Mockito.when;
 import java.io.File;
@@ -218,6 +219,14 @@ public class FileSystemArchiveTest {
         equalTo(false));
   }
 
+  @Test
+  public void deleteFling_zipDiskNotFound_noThrow() throws IOException {
+    assertThat(Files.exists(tempDir.resolve(artifactEntity2.getFling().getId() + ".zip")),
+        equalTo(false));
+
+    assertDoesNotThrow(() -> fileSystemArchive.deleteFling(artifactEntity2.getFling().getId()));
+  }
+
   private void repopulateArchivePath() throws IOException, URISyntaxException {
     Files.walkFileTree(tempDir, new SimpleFileVisitor<Path>() {
       @Override

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

@@ -18,7 +18,7 @@ export default function FlingAdmin() {
 
   useEffect(() => {
     if (flingId) {
-      dispatch(setActiveFling(flingId))
+      dispatch(setActiveFling(flingId));
     }
   }, [flingId, dispatch]);
 

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

@@ -3,10 +3,26 @@ import React, { useRef } from 'react';
 import classNames from 'classnames';
 import { NavLink } from "react-router-dom";
 
-import { flingClient } from '../../util/flingclient';
+import { useSelector, useDispatch } from "react-redux";
+
+import { deleteFling } from "../../redux/actions";
 
 function TileAction(props) {
   let shareUrlRef = useRef(null);
+  const dispatch = useDispatch();
+
+  function copyShareUrl() {
+    shareUrlRef.current.focus();
+    shareUrlRef.current.select();
+
+    try {
+      let successful = document.execCommand('copy');
+      let msg = successful ? 'successful' : 'unsuccessful';
+      console.log('Copying to clipoard ' + msg);
+    } catch (err) {
+      log.error("Couldn't copy to clipboard: ", err);
+    }
+  }
 
   return (
     <div className="tile-action dropdown">
@@ -27,8 +43,7 @@ function TileAction(props) {
         <li className="menu-item">
           <div className="form-group">
             <label className="form-switch">
-              <input type="checkbox"
+              <input type="checkbox" checked={props.fling.shared} />
-                checked={props.fling.shared} onChange={toggleShared} />
               <i className="form-icon" />
               {props.fling.shared ? "Shared" : "Private"}
             </label>
@@ -36,7 +51,7 @@ function TileAction(props) {
         </li>
         <li className="menu-item">
           <button className="btn btn-link text-warning pl-0"
-            onClick={deleteFling}>
+            onClick={ev => dispatch(deleteFling(props.fling.id))}>
             <i className="icon icon-delete mr-1" /> Remove
           </button>
         </li>
@@ -44,34 +59,14 @@ function TileAction(props) {
     </div>
   );
 
-  function copyShareUrl() {
-    shareUrlRef.current.focus();
-    shareUrlRef.current.select();
-
-    try {
-      let successful = document.execCommand('copy');
-      let msg = successful ? 'successful' : 'unsuccessful';
-      console.log('Copying to clipoard ' + msg);
-    } catch (err) {
-      log.error("Couldn't copy to clipboard: ", err);
-    }
-  }
-
-  async function deleteFling() {
-    await flingClient.deleteFling(props.fling.id);
-    await props.refreshFlingListFn();
-  }
-
-  async function toggleShared() {
-    await flingClient.putFling(props.fling.id, { "sharing": { "shared": !props.fling.shared } });
-    await props.refreshFlingListFn();
-  }
 }
 
 export default function FlingTile(props) {
+  const activeFling = useSelector((state) => state.flings.activeFling);
+
   let tileClasses = classNames(
     "tile", "tile-centered", "p-2", "c-hand",
-    { "active": props.activeFling === props.fling.id }
+    { "active": activeFling ? activeFling.id === props.fling.id : false }
   );
 
   return (
@@ -81,11 +76,13 @@ export default function FlingTile(props) {
           <NavLink to={`/admin/${props.fling.id}`}>
             <div className="tile-title">{props.fling.name}</div>
             <small className="tile-subtitle text-gray">
-              14MB · Public · 1 Jan, 2017
+              {`${props.fling.shared ? "Shared" : "Private"}` +
+                ` · ${(new Date(props.fling.creationTime)).toLocaleDateString()}` +
+                ` · ${props.fling.authCode ? "Protected" : "Unprotected"}`}
             </small>
           </NavLink>
         </div>
-        <TileAction fling={props.fling} refreshFlingListFn={props.refreshFlingListFn} />
+        <TileAction fling={props.fling} />
       </div>
     </div>
   );

web/fling/src/redux/actionTypes.js

@@ -1,3 +1,4 @@
 export const SET_FLINGS = "SET_FLINGS";
 export const ADD_FLING = "ADD_FLING";
+export const DELETE_FLING = "DELETE_FLING";
 export const SET_ACTIVE_FLING = "SET_ACTIVE_FLING";

web/fling/src/redux/actions.js

@@ -24,15 +24,14 @@ function setActiveFlingAction(fling) {
     }
 }
 
-
 function setActiveFling(id) {
     return (dispatch, getState) => {
         if (!id) {
             log.debug(`Not setting active Fling. No id given.`);
-            return;
+            return false;
         }
         const { flings: { flings } } = getState();
-        let foundFling = flings.find(f => f.id === id);
+        const foundFling = flings.find(f => f.id === id);
 
         if (foundFling) {
             log.info(`Found active fling ${id} in local storage`);
@@ -48,7 +47,7 @@ function setActiveFling(id) {
                     dispatch(setActiveFlingAction(fling))
                 })
                 .catch(error => {
-                    log.warn(`Could not find active fling. ` +
+                    log.warn(`Could not find active fling: ${error} \n` +
                         `Resetting active fling`);
                     dispatch(setActiveFlingAction(undefined));
                 })
@@ -57,11 +56,34 @@ function setActiveFling(id) {
 }
 
 function retrieveFlings() {
-    return (dispatch) => {
+    return (dispatch, getState) => {
-        let flingClient = new FlingClient();
+        const { flings: { activeFling } } = getState();
+        const flingClient = new FlingClient();
+
         flingClient.getFlings()
-            .then(flings => dispatch(setFlingsAction(flings)));
+            .then(flings => {
+                dispatch(setFlingsAction(flings));
+                if (activeFling) {
+                    dispatch(setActiveFling(activeFling.id));
+                }
+            });
     }
 }
 
-export { retrieveFlings, setActiveFling };
+function deleteFling(id) {
+    return (dispatch, getState) => {
+        if (!id) {
+            log.debug(`Not deleting Fling. No id given.`);
+            return;
+        }
+
+        const flingClient = new FlingClient();
+
+        flingClient.deleteFling(id)
+            .then(() => dispatch(retrieveFlings()))
+            .catch(error =>
+                log.error(`Could not delete fling ${id}: ${error}`));
+    }
+}
+
+export { retrieveFlings, setActiveFling, deleteFling };

web/fling/src/redux/reducers/flings.js

@@ -1,3 +1,4 @@
+import log from "loglevel";
 import produce from "immer";
 
 import { SET_FLINGS, SET_ACTIVE_FLING, ADD_FLING } from "../actionTypes";
@@ -16,7 +17,19 @@ export default produce((draft, action) => {
             draft.flings = action.payload;
             break;
         case ADD_FLING:
+            // Check storage again here, otherwise there could be a race
+            // condition due to async calls of SET_FLINGS and ADD_FLING
+            let foundFlingIdx = draft.flings.findIndex(fling =>
+                fling.id === action.payload.id);
+
+            if (foundFlingIdx === -1) {
+                log.debug(`Adding new fling with id ${action.payload.id}`)
                 draft.flings.push(action.payload);
+            } else {
+                log.debug(`Fling already exists. ` +
+                    `Updating fling with id ${action.payload.id}`)
+                draft.flings[foundFlingIdx] = action.payload
+            }
             break;
         case SET_ACTIVE_FLING:
             draft.activeFling = action.payload;

web/fling/src/redux/selectorTypes.js

@@ -1,3 +0,0 @@
-export const FLING_FILTERS = {
-    ALL: "all"
-}

web/fling/src/redux/selectors/flingSelectors.js

@@ -1,10 +0,0 @@
-import { FLING_FILTERS } from "../selectorTypes";
-
-export const flingSelector = (store, flingFilter) => {
-    switch(flingFilter) {
-        case FLING_FILTERS.ALL:
-            return store.flings.flings;
-        default:
-            return [];
-    }
-}