From fc4f00685875a2ade02780d97031a4ec81c5711a Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Thu, 19 Apr 2018 20:32:12 +0200
Subject: [PATCH 01/23] Missing previous changelog

---
 changes/changelog.d/165.doc | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changes/changelog.d/165.doc

diff --git a/changes/changelog.d/165.doc b/changes/changelog.d/165.doc
new file mode 100644
index 000000000..4825af09b
--- /dev/null
+++ b/changes/changelog.d/165.doc
@@ -0,0 +1 @@
+Better documentation for hardware requirements and memory usage (#165)

From 1f349c82cf023908f9b5d6d51219779b5701c146 Mon Sep 17 00:00:00 2001
From: Bat <baptiste@gelez.xyz>
Date: Thu, 19 Apr 2018 19:57:12 +0100
Subject: [PATCH 02/23] Translate instance activity

---
 front/src/views/instance/Timeline.vue | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/front/src/views/instance/Timeline.vue b/front/src/views/instance/Timeline.vue
index f4ddedfb7..8b34798cd 100644
--- a/front/src/views/instance/Timeline.vue
+++ b/front/src/views/instance/Timeline.vue
@@ -2,10 +2,10 @@
   <div class="main pusher" v-title="'Instance Timeline'">
     <div class="ui vertical center aligned stripe segment">
       <div v-if="isLoading" :class="['ui', {'active': isLoading}, 'inverted', 'dimmer']">
-        <div class="ui text loader">Loading timeline...</div>
+        <div class="ui text loader">{{ $t('Loading timeline...') }}</div>
       </div>
       <div v-else class="ui text container">
-        <h1 class="ui header">Recent activity on this instance</h1>
+        <h1 class="ui header">{{ $t('Recent activity on this instance') }}</h1>
         <div class="ui feed">
           <component
             class="event"

From 0446a2b0faef031cef81ff3980cf73e65d5c6713 Mon Sep 17 00:00:00 2001
From: Bat <baptiste@gelez.xyz>
Date: Thu, 19 Apr 2018 20:09:01 +0100
Subject: [PATCH 03/23] i18n: federation views

---
 front/src/views/federation/Base.vue           |  8 +++---
 front/src/views/federation/LibraryDetail.vue  | 26 +++++++++----------
 .../views/federation/LibraryFollowersList.vue |  5 ++--
 front/src/views/federation/LibraryList.vue    | 16 ++++++------
 .../src/views/federation/LibraryTrackList.vue |  2 +-
 5 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/front/src/views/federation/Base.vue b/front/src/views/federation/Base.vue
index 7958bb36b..951fe9f0f 100644
--- a/front/src/views/federation/Base.vue
+++ b/front/src/views/federation/Base.vue
@@ -3,16 +3,16 @@
     <div class="ui secondary pointing menu">
       <router-link
         class="ui item"
-        :to="{name: 'federation.libraries.list'}">Libraries</router-link>
+        :to="{name: 'federation.libraries.list'}">{{ $t('Libraries') }}</router-link>
       <router-link
         class="ui item"
-        :to="{name: 'federation.tracks.list'}">Tracks</router-link>
+        :to="{name: 'federation.tracks.list'}">{{ $t('Tracks') }}</router-link>
         <div class="ui secondary right menu">
           <router-link
             class="ui item"
             :to="{name: 'federation.followers.list'}">
-            Followers
-            <div class="ui teal label" title="Pending requests">{{ requestsCount }}</div>
+            {{ $t('Followers') }}
+            <div class="ui teal label" :title="$t('Pending requests')">{{ requestsCount }}</div>
           </router-link>
         </div>
     </div>
diff --git a/front/src/views/federation/LibraryDetail.vue b/front/src/views/federation/LibraryDetail.vue
index 20250e333..bd2e63c4d 100644
--- a/front/src/views/federation/LibraryDetail.vue
+++ b/front/src/views/federation/LibraryDetail.vue
@@ -19,18 +19,18 @@
             <tbody>
               <tr>
                 <td >
-                  Follow status
+                  {{ $t('Follow status') }}
                   <span :data-tooltip="$t('This indicate if the remote library granted you access')"><i class="question circle icon"></i></span>
                 </td>
                 <td>
                   <template v-if="object.follow.approved === null">
-                    <i class="loading icon"></i> Pending approval
+                    <i class="loading icon"></i> {{ $t('Pending approval') }}
                   </template>
                   <template v-else-if="object.follow.approved === true">
-                    <i class="check icon"></i> Following
+                    <i class="check icon"></i> {{ $t('Following') }}
                   </template>
                   <template v-else-if="object.follow.approved === false">
-                    <i class="x icon"></i> Not following
+                    <i class="x icon"></i> {{ $t('Not following') }}
                   </template>
                 </td>
                 <td>
@@ -38,7 +38,7 @@
               </tr>
               <tr>
                 <td>
-                  Federation
+                  {{ $t('Federation') }}
                   <span :data-tooltip="$t('Use this flag to enable/disable federation with this library')"><i class="question circle icon"></i></span>
                 </td>
                 <td>
@@ -54,7 +54,7 @@
               </tr>
               <tr>
                 <td>
-                  Auto importing
+                  {{ $t('Auto importing') }}
                   <span :data-tooltip="$t('When enabled, auto importing will automatically import new tracks published in this library')"><i class="question circle icon"></i></span>
                 </td>
                 <td>
@@ -82,14 +82,14 @@
               </tr>
               -->
               <tr>
-                <td>Library size</td>
+                <td>{{ $t('Library size') }}</td>
                 <td>
-                  {{ object.tracks_count }} tracks
+                  {{ $t('{%count%} tracks', { count: object.tracks_count }) }}
                 </td>
                 <td></td>
               </tr>
               <tr>
-                <td>Last fetched</td>
+                <td>{{ $t('Last fetched') }}</td>
                 <td>
                   <human-date v-if="object.fetched_date" :date="object.fetched_date"></human-date>
                   <template v-else>Never</template>
@@ -97,10 +97,10 @@
                     @click="scan"
                     v-if="!scanTrigerred"
                     :class="['ui', 'basic', {loading: isScanLoading}, 'button']">
-                    <i class="sync icon"></i> Trigger scan
+                    <i class="sync icon"></i> {{ $t('Trigger scan') }}
                   </button>
                   <button v-else class="ui success button">
-                    <i class="check icon"></i> Scan triggered!
+                    <i class="check icon"></i> {{ $t('Scan triggered!') }}
                   </button>
 
                 </td>
@@ -110,10 +110,10 @@
           </table>
         </div>
         <div class="ui hidden divider"></div>
-        <button @click="fetchData" class="ui basic button">Refresh</button>
+        <button @click="fetchData" class="ui basic button">{{ $t('Refresh') }}</button>
       </div>
       <div class="ui vertical stripe segment">
-        <h2>Tracks available in this library</h2>
+        <h2>{{ $t('Tracks available in this library') }}</h2>
         <library-track-table v-if="!isLoading" :filters="{library: id}"></library-track-table>
       </div>
     </template>
diff --git a/front/src/views/federation/LibraryFollowersList.vue b/front/src/views/federation/LibraryFollowersList.vue
index 8ca120e8b..ce79e4787 100644
--- a/front/src/views/federation/LibraryFollowersList.vue
+++ b/front/src/views/federation/LibraryFollowersList.vue
@@ -1,10 +1,9 @@
 <template>
   <div v-title="'Followers'">
     <div class="ui vertical stripe segment">
-      <h2 class="ui header">Browsing followers</h2>
+      <h2 class="ui header">{{ $t('Browsing followers') }}</h2>
       <p>
-        Be careful when accepting follow requests, as it means the follower
-        will have access to your entire library.
+        {{ $t('Be careful when accepting follow requests, as it means the follower will have access to your entire library.') }}
       </p>
       <div class="ui hidden divider"></div>
       <library-follow-table></library-follow-table>
diff --git a/front/src/views/federation/LibraryList.vue b/front/src/views/federation/LibraryList.vue
index 43800a72e..7b0b25941 100644
--- a/front/src/views/federation/LibraryList.vue
+++ b/front/src/views/federation/LibraryList.vue
@@ -1,22 +1,22 @@
 <template>
   <div v-title="'Libraries'">
     <div class="ui vertical stripe segment">
-      <h2 class="ui header">Browsing libraries</h2>
+      <h2 class="ui header">{{ $t('Browsing libraries') }}</h2>
       <router-link
         class="ui basic green button"
         :to="{name: 'federation.libraries.scan'}">
         <i class="plus icon"></i>
-        Add a new library
+        {{ $t('Add a new library') }}
       </router-link>
       <div class="ui hidden divider"></div>
       <div :class="['ui', {'loading': isLoading}, 'form']">
         <div class="fields">
           <div class="field">
-            <label>Search</label>
+            <label>{{ $t('Search') }}</label>
             <input type="text" v-model="query" placeholder="Enter an library domain name..."/>
           </div>
           <div class="field">
-            <label>Ordering</label>
+            <label>{{ $t('Ordering') }}</label>
             <select class="ui dropdown" v-model="ordering">
               <option v-for="option in orderingOptions" :value="option[0]">
                 {{ option[1] }}
@@ -24,14 +24,14 @@
             </select>
           </div>
           <div class="field">
-            <label>Ordering direction</label>
+            <label>{{ $t('Ordering direction') }}</label>
             <select class="ui dropdown" v-model="orderingDirection">
-              <option value="">Ascending</option>
-              <option value="-">Descending</option>
+              <option value="">{{ $t('Ascending') }}</option>
+              <option value="-">{{ $t('Descending') }}</option>
             </select>
           </div>
           <div class="field">
-            <label>Results per page</label>
+            <label>{{ $t('Results per page') }}</label>
             <select class="ui dropdown" v-model="paginateBy">
               <option :value="parseInt(12)">12</option>
               <option :value="parseInt(25)">25</option>
diff --git a/front/src/views/federation/LibraryTrackList.vue b/front/src/views/federation/LibraryTrackList.vue
index 42526b6e4..b9a78527f 100644
--- a/front/src/views/federation/LibraryTrackList.vue
+++ b/front/src/views/federation/LibraryTrackList.vue
@@ -1,7 +1,7 @@
 <template>
   <div v-title="'Federated tracks'">
     <div class="ui vertical stripe segment">
-      <h2 class="ui header">Browsing federated tracks</h2>
+      <h2 class="ui header">{{ $t('Browsing federated tracks') }}</h2>
       <div class="ui hidden divider"></div>
       <library-track-table :show-library="true"></library-track-table>
     </div>

From ced851891d1276143e18eac038948a58f91f3772 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Thu, 19 Apr 2018 21:19:40 +0200
Subject: [PATCH 04/23] Fix #163: Avoid downloading audio files multiple times
 from remote libraries

---
 api/funkwhale_api/music/views.py    | 27 ++++++++++++++++++---------
 changes/changelog.d/163.enhancement |  1 +
 2 files changed, 19 insertions(+), 9 deletions(-)
 create mode 100644 changes/changelog.d/163.enhancement

diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py
index e8ace1b3a..f961e3fdf 100644
--- a/api/funkwhale_api/music/views.py
+++ b/api/funkwhale_api/music/views.py
@@ -23,13 +23,14 @@ from rest_framework import permissions
 from musicbrainzngs import ResponseError
 
 from funkwhale_api.common import utils as funkwhale_utils
-from funkwhale_api.federation import actors
-from funkwhale_api.requests.models import ImportRequest
-from funkwhale_api.musicbrainz import api
 from funkwhale_api.common.permissions import (
     ConditionalAuthentication, HasModelPermission)
 from taggit.models import Tag
+from funkwhale_api.federation import actors
 from funkwhale_api.federation.authentication import SignatureAuthentication
+from funkwhale_api.federation.models import LibraryTrack
+from funkwhale_api.musicbrainz import api
+from funkwhale_api.requests.models import ImportRequest
 
 from . import filters
 from . import forms
@@ -195,12 +196,13 @@ class TrackFileViewSet(viewsets.ReadOnlyModelViewSet):
 
     @detail_route(methods=['get'])
     def serve(self, request, *args, **kwargs):
+        queryset = models.TrackFile.objects.select_related(
+            'library_track',
+            'track__album__artist',
+            'track__artist',
+        )
         try:
-            f = models.TrackFile.objects.select_related(
-                'library_track',
-                'track__album__artist',
-                'track__artist',
-            ).get(pk=kwargs['pk'])
+            f = queryset.get(pk=kwargs['pk'])
         except models.TrackFile.DoesNotExist:
             return Response(status=404)
 
@@ -213,7 +215,14 @@ class TrackFileViewSet(viewsets.ReadOnlyModelViewSet):
         if library_track and not audio_file:
             if not library_track.audio_file:
                 # we need to populate from cache
-                library_track.download_audio()
+                with transaction.atomic():
+                    # why the transaction/select_for_update?
+                    # this is because browsers may send multiple requests
+                    # in a short time range, for partial content,
+                    # thus resulting in multiple downloads from the remote
+                    qs = LibraryTrack.objects.select_for_update()
+                    library_track = qs.get(pk=library_track.pk)
+                    library_track.download_audio()
             audio_file = library_track.audio_file
             mt = library_track.audio_mimetype
         response = Response()
diff --git a/changes/changelog.d/163.enhancement b/changes/changelog.d/163.enhancement
new file mode 100644
index 000000000..89225eb9c
--- /dev/null
+++ b/changes/changelog.d/163.enhancement
@@ -0,0 +1 @@
+Avoid downloading audio files multiple times from remote libraries (#163)

From de4b0f3022e222c9260f786a64e6e41d759220b4 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Thu, 19 Apr 2018 22:15:13 +0200
Subject: [PATCH 05/23] Fix #124 and #155: properly reset everything on logout

---
 changes/changelog.d/124.bugfix            |  1 +
 changes/changelog.d/155.bugfix            |  1 +
 front/src/components/Sidebar.vue          |  2 +-
 front/src/store/auth.js                   | 27 +++++++++++++++++------
 front/src/store/favorites.js              |  4 ++++
 front/src/store/player.js                 |  4 ++++
 front/src/store/playlists.js              |  5 +++++
 front/src/store/queue.js                  |  6 +++++
 front/src/store/radios.js                 |  4 ++++
 front/test/unit/specs/store/auth.spec.js  | 12 +++++-----
 front/test/unit/specs/store/queue.spec.js |  2 +-
 11 files changed, 54 insertions(+), 14 deletions(-)
 create mode 100644 changes/changelog.d/124.bugfix
 create mode 100644 changes/changelog.d/155.bugfix

diff --git a/changes/changelog.d/124.bugfix b/changes/changelog.d/124.bugfix
new file mode 100644
index 000000000..b1e104a43
--- /dev/null
+++ b/changes/changelog.d/124.bugfix
@@ -0,0 +1 @@
+Reset all sensitive front-end data on logout (#124)
diff --git a/changes/changelog.d/155.bugfix b/changes/changelog.d/155.bugfix
new file mode 100644
index 000000000..2252d56d7
--- /dev/null
+++ b/changes/changelog.d/155.bugfix
@@ -0,0 +1 @@
+Fixed broken playlist modal after login (#155)
diff --git a/front/src/components/Sidebar.vue b/front/src/components/Sidebar.vue
index 96047ab98..56c63adbd 100644
--- a/front/src/components/Sidebar.vue
+++ b/front/src/components/Sidebar.vue
@@ -35,7 +35,7 @@
         <router-link class="item" v-if="$store.state.auth.authenticated" :to="{name: 'logout'}"><i class="sign out icon"></i> Logout</router-link>
         <router-link class="item" v-else :to="{name: 'login'}"><i class="sign in icon"></i> Login</router-link>
         <router-link class="item" :to="{path: '/library'}"><i class="sound icon"> </i>Browse library</router-link>
-        <router-link class="item" :to="{path: '/favorites'}"><i class="heart icon"></i> Favorites</router-link>
+        <router-link class="item" v-if="$store.state.auth.authenticated" :to="{path: '/favorites'}"><i class="heart icon"></i> Favorites</router-link>
         <a
           @click="$store.commit('playlists/chooseTrack', null)"
           v-if="$store.state.auth.authenticated"
diff --git a/front/src/store/auth.js b/front/src/store/auth.js
index e72e1968f..b1753404f 100644
--- a/front/src/store/auth.js
+++ b/front/src/store/auth.js
@@ -19,6 +19,14 @@ export default {
     }
   },
   mutations: {
+    reset (state) {
+      state.authenticated = false
+      state.profile = null
+      state.username = ''
+      state.token = ''
+      state.tokenData = {}
+      state.availablePermissions = {}
+    },
     profile: (state, value) => {
       state.profile = value
     },
@@ -53,8 +61,6 @@ export default {
       return axios.post('token/', credentials).then(response => {
         logger.default.info('Successfully logged in as', credentials.username)
         commit('token', response.data.token)
-        commit('username', credentials.username)
-        commit('authenticated', true)
         dispatch('fetchProfile')
         // Redirect to a specified route
         router.push(next)
@@ -64,19 +70,25 @@ export default {
       })
     },
     logout ({commit}) {
-      commit('authenticated', false)
+      let modules = [
+        'auth',
+        'favorites',
+        'player',
+        'playlists',
+        'queue',
+        'radios'
+      ]
+      modules.forEach(m => {
+        commit(`${m}/reset`, null, {root: true})
+      })
       logger.default.info('Log out, goodbye!')
       router.push({name: 'index'})
     },
     check ({commit, dispatch, state}) {
       logger.default.info('Checking authentication...')
       var jwt = state.token
-      var username = state.username
       if (jwt) {
-        commit('authenticated', true)
-        commit('username', username)
         commit('token', jwt)
-        logger.default.info('Logged back in as ' + username)
         dispatch('fetchProfile')
         dispatch('refreshToken')
       } else {
@@ -88,6 +100,7 @@ export default {
       return axios.get('users/users/me/').then((response) => {
         logger.default.info('Successfully fetched user profile')
         let data = response.data
+        commit('authenticated', true)
         commit('profile', data)
         commit('username', data.username)
         dispatch('favorites/fetch', null, {root: true})
diff --git a/front/src/store/favorites.js b/front/src/store/favorites.js
index a4f85b235..b7e789511 100644
--- a/front/src/store/favorites.js
+++ b/front/src/store/favorites.js
@@ -20,6 +20,10 @@ export default {
         }
       }
       state.count = state.tracks.length
+    },
+    reset (state) {
+      state.tracks = []
+      state.count = 0
     }
   },
   getters: {
diff --git a/front/src/store/player.js b/front/src/store/player.js
index d849b7b56..ed437c3f0 100644
--- a/front/src/store/player.js
+++ b/front/src/store/player.js
@@ -15,6 +15,10 @@ export default {
     looping: 0 // 0 -> no, 1 -> on  track, 2 -> on queue
   },
   mutations: {
+    reset (state) {
+      state.errorCount = 0
+      state.playing = false
+    },
     volume (state, value) {
       value = parseFloat(value)
       value = Math.min(value, 1)
diff --git a/front/src/store/playlists.js b/front/src/store/playlists.js
index b3ed3ab23..d0e144d80 100644
--- a/front/src/store/playlists.js
+++ b/front/src/store/playlists.js
@@ -17,6 +17,11 @@ export default {
     },
     showModal (state, value) {
       state.showModal = value
+    },
+    reset (state) {
+      state.playlists = []
+      state.modalTrack = null
+      state.showModal = false
     }
   },
   actions: {
diff --git a/front/src/store/queue.js b/front/src/store/queue.js
index 6a26fa1e9..2890dd1e8 100644
--- a/front/src/store/queue.js
+++ b/front/src/store/queue.js
@@ -10,6 +10,12 @@ export default {
     previousQueue: null
   },
   mutations: {
+    reset (state) {
+      state.tracks = []
+      state.currentIndex = -1
+      state.ended = true
+      state.previousQueue = null
+    },
     currentIndex (state, value) {
       state.currentIndex = value
     },
diff --git a/front/src/store/radios.js b/front/src/store/radios.js
index e95db5126..49bbd4f94 100644
--- a/front/src/store/radios.js
+++ b/front/src/store/radios.js
@@ -26,6 +26,10 @@ export default {
     }
   },
   mutations: {
+    reset (state) {
+      state.running = false
+      state.current = false
+    },
     current: (state, value) => {
       state.current = value
     },
diff --git a/front/test/unit/specs/store/auth.spec.js b/front/test/unit/specs/store/auth.spec.js
index 518dc10d4..6b187f5c6 100644
--- a/front/test/unit/specs/store/auth.spec.js
+++ b/front/test/unit/specs/store/auth.spec.js
@@ -89,7 +89,12 @@ describe('store/auth', () => {
         action: store.actions.logout,
         params: {state: {}},
         expectedMutations: [
-          { type: 'authenticated', payload: false }
+          { type: 'auth/reset', payload: null, options: {root: true} },
+          { type: 'favorites/reset', payload: null, options: {root: true} },
+          { type: 'player/reset', payload: null, options: {root: true} },
+          { type: 'playlists/reset', payload: null, options: {root: true} },
+          { type: 'queue/reset', payload: null, options: {root: true} },
+          { type: 'radios/reset', payload: null, options: {root: true} }
         ]
       }, done)
     })
@@ -107,8 +112,6 @@ describe('store/auth', () => {
         action: store.actions.check,
         params: {state: {token: 'test', username: 'user'}},
         expectedMutations: [
-          { type: 'authenticated', payload: true },
-          { type: 'username', payload: 'user' },
           { type: 'token', payload: 'test' }
         ],
         expectedActions: [
@@ -132,8 +135,6 @@ describe('store/auth', () => {
         payload: {credentials: credentials},
         expectedMutations: [
           { type: 'token', payload: 'test' },
-          { type: 'username', payload: 'bob' },
-          { type: 'authenticated', payload: true }
         ],
         expectedActions: [
           { type: 'fetchProfile' }
@@ -175,6 +176,7 @@ describe('store/auth', () => {
       testAction({
         action: store.actions.fetchProfile,
         expectedMutations: [
+          { type: 'authenticated', payload: true },
           { type: 'profile', payload: profile },
           { type: 'username', payload: profile.username },
           { type: 'permission', payload: {key: 'admin', status: true} }
diff --git a/front/test/unit/specs/store/queue.spec.js b/front/test/unit/specs/store/queue.spec.js
index 2bc5cde4e..5304d1d75 100644
--- a/front/test/unit/specs/store/queue.spec.js
+++ b/front/test/unit/specs/store/queue.spec.js
@@ -326,7 +326,7 @@ describe('store/queue', () => {
         action: store.actions.shuffle,
         params: {state: {currentIndex: 1, tracks: tracks}},
         expectedMutations: [
-          { type: 'player/currentTime', payload: 0 , options: {root: true}},
+          { type: 'player/currentTime', payload: 0, options: {root: true}},
           { type: 'tracks', payload: [] }
         ],
         expectedActions: [

From 2be46b3320dc0b54d4f5025ea38f9c3be7c0d0ca Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Thu, 19 Apr 2018 22:25:58 +0200
Subject: [PATCH 06/23] Fix #142: Increased max_length on TrackFile.source

---
 .../migrations/0025_auto_20180419_2023.py      | 18 ++++++++++++++++++
 api/funkwhale_api/music/models.py              |  2 +-
 changes/changelog.d/142.enhancement            |  1 +
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 api/funkwhale_api/music/migrations/0025_auto_20180419_2023.py
 create mode 100644 changes/changelog.d/142.enhancement

diff --git a/api/funkwhale_api/music/migrations/0025_auto_20180419_2023.py b/api/funkwhale_api/music/migrations/0025_auto_20180419_2023.py
new file mode 100644
index 000000000..6b0230d50
--- /dev/null
+++ b/api/funkwhale_api/music/migrations/0025_auto_20180419_2023.py
@@ -0,0 +1,18 @@
+# Generated by Django 2.0.3 on 2018-04-19 20:23
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('music', '0024_populate_uuid'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='trackfile',
+            name='source',
+            field=models.URLField(blank=True, max_length=500, null=True),
+        ),
+    ]
diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py
index 4ec3ff427..cc3849d50 100644
--- a/api/funkwhale_api/music/models.py
+++ b/api/funkwhale_api/music/models.py
@@ -412,7 +412,7 @@ class TrackFile(models.Model):
     track = models.ForeignKey(
         Track, related_name='files', on_delete=models.CASCADE)
     audio_file = models.FileField(upload_to='tracks/%Y/%m/%d', max_length=255)
-    source = models.URLField(null=True, blank=True)
+    source = models.URLField(null=True, blank=True, max_length=500)
     creation_date = models.DateTimeField(default=timezone.now)
     modification_date = models.DateTimeField(auto_now=True)
     duration = models.IntegerField(null=True, blank=True)
diff --git a/changes/changelog.d/142.enhancement b/changes/changelog.d/142.enhancement
new file mode 100644
index 000000000..9784f5403
--- /dev/null
+++ b/changes/changelog.d/142.enhancement
@@ -0,0 +1 @@
+Increased max_length on TrackFile.source, this will help when importing files with a really long path (#142)

From a210cfcb9b9ed2179f340226524c45527d2ed5c5 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Fri, 20 Apr 2018 18:41:17 +0200
Subject: [PATCH 07/23] Fix #169: Fixed template syntax error

---
 front/src/components/playlists/PlaylistModal.vue | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/front/src/components/playlists/PlaylistModal.vue b/front/src/components/playlists/PlaylistModal.vue
index 03e36ee88..404948dc0 100644
--- a/front/src/components/playlists/PlaylistModal.vue
+++ b/front/src/components/playlists/PlaylistModal.vue
@@ -48,7 +48,7 @@
                 <div
                   v-if="track"
                   class="ui green icon basic small right floated button"
-                  :title="{{ $t('Add to this playlist') }}"
+                  :title="$t('Add to this playlist')"
                   @click="addToPlaylist(playlist.id)">
                   <i class="plus icon"></i> {{ $t('Add track') }}
                 </div>

From a4ce093919f5456514877d89e57418ff1f292972 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Fri, 20 Apr 2018 18:41:29 +0200
Subject: [PATCH 08/23] Fix lint errors

---
 front/test/unit/specs/store/auth.spec.js   | 4 ++--
 front/test/unit/specs/store/player.spec.js | 4 ++--
 front/test/unit/specs/store/queue.spec.js  | 2 +-
 front/test/unit/specs/store/radios.spec.js | 1 -
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/front/test/unit/specs/store/auth.spec.js b/front/test/unit/specs/store/auth.spec.js
index 6b187f5c6..3d175e9f9 100644
--- a/front/test/unit/specs/store/auth.spec.js
+++ b/front/test/unit/specs/store/auth.spec.js
@@ -134,7 +134,7 @@ describe('store/auth', () => {
         action: store.actions.login,
         payload: {credentials: credentials},
         expectedMutations: [
-          { type: 'token', payload: 'test' },
+          { type: 'token', payload: 'test' }
         ],
         expectedActions: [
           { type: 'fetchProfile' }
@@ -183,7 +183,7 @@ describe('store/auth', () => {
         ],
         expectedActions: [
           { type: 'favorites/fetch', payload: null, options: {root: true} },
-          { type: 'playlists/fetchOwn', payload: null, options: {root: true} },
+          { type: 'playlists/fetchOwn', payload: null, options: {root: true} }
         ]
       }, done)
     })
diff --git a/front/test/unit/specs/store/player.spec.js b/front/test/unit/specs/store/player.spec.js
index b55fb010d..1e6c9b33a 100644
--- a/front/test/unit/specs/store/player.spec.js
+++ b/front/test/unit/specs/store/player.spec.js
@@ -132,7 +132,7 @@ describe('store/player', () => {
       testAction({
         action: store.actions.trackEnded,
         payload: {test: 'track'},
-        params: {rootState: {queue: {currentIndex:0, tracks: [1, 2]}}},
+        params: {rootState: {queue: {currentIndex: 0, tracks: [1, 2]}}},
         expectedActions: [
           { type: 'trackListened', payload: {test: 'track'} },
           { type: 'queue/next', payload: null, options: {root: true} }
@@ -143,7 +143,7 @@ describe('store/player', () => {
       testAction({
         action: store.actions.trackEnded,
         payload: {test: 'track'},
-        params: {rootState: {queue: {currentIndex:1, tracks: [1, 2]}}},
+        params: {rootState: {queue: {currentIndex: 1, tracks: [1, 2]}}},
         expectedActions: [
           { type: 'trackListened', payload: {test: 'track'} },
           { type: 'radios/populateQueue', payload: null, options: {root: true} },
diff --git a/front/test/unit/specs/store/queue.spec.js b/front/test/unit/specs/store/queue.spec.js
index 5304d1d75..0df7608e7 100644
--- a/front/test/unit/specs/store/queue.spec.js
+++ b/front/test/unit/specs/store/queue.spec.js
@@ -326,7 +326,7 @@ describe('store/queue', () => {
         action: store.actions.shuffle,
         params: {state: {currentIndex: 1, tracks: tracks}},
         expectedMutations: [
-          { type: 'player/currentTime', payload: 0, options: {root: true}},
+          { type: 'player/currentTime', payload: 0, options: {root: true} },
           { type: 'tracks', payload: [] }
         ],
         expectedActions: [
diff --git a/front/test/unit/specs/store/radios.spec.js b/front/test/unit/specs/store/radios.spec.js
index 6de6b8dd9..3a8c48f04 100644
--- a/front/test/unit/specs/store/radios.spec.js
+++ b/front/test/unit/specs/store/radios.spec.js
@@ -97,6 +97,5 @@ describe('store/radios', () => {
         expectedActions: []
       }, done)
     })
-
   })
 })

From 083b7b8e2fb86ab4459bd1a9b603eddf31f6d49f Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Fri, 20 Apr 2018 18:46:18 +0200
Subject: [PATCH 09/23] Added one basic unittest for component

---
 front/test/unit/specs/components/common.spec.js | 10 ++++++++++
 front/test/unit/utils.js                        |  7 +++++++
 2 files changed, 17 insertions(+)
 create mode 100644 front/test/unit/specs/components/common.spec.js

diff --git a/front/test/unit/specs/components/common.spec.js b/front/test/unit/specs/components/common.spec.js
new file mode 100644
index 000000000..1af4144ca
--- /dev/null
+++ b/front/test/unit/specs/components/common.spec.js
@@ -0,0 +1,10 @@
+import Username from '@/components/common/Username.vue'
+
+import { render } from '../../utils'
+
+describe('Username', () => {
+  it('displays username', () => {
+    const vm = render(Username, {username: 'Hello'})
+    expect(vm.$el.textContent).to.equal('Hello')
+  })
+})
diff --git a/front/test/unit/utils.js b/front/test/unit/utils.js
index 233ee982e..6471fc97f 100644
--- a/front/test/unit/utils.js
+++ b/front/test/unit/utils.js
@@ -1,4 +1,11 @@
 // helper for testing action with expected mutations
+import Vue from 'vue'
+
+export const render = (Component, propsData) => {
+  const Constructor = Vue.extend(Component)
+  return new Constructor({ propsData: propsData }).$mount()
+}
+
 export const testAction = ({action, payload, params, expectedMutations, expectedActions}, done) => {
   let mutationsCount = 0
   let actionsCount = 0

From 84c4e5ebbbc7f9cf6b6bce6b0cfec114e88d1db7 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Fri, 20 Apr 2018 19:25:29 +0200
Subject: [PATCH 10/23] Added CI check to catch build errors

---
 .gitlab-ci.yml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 217047794..b2078f153 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -46,6 +46,7 @@ test_front:
 
   script:
     - yarn install
+    - yarn run build | tee /dev/stderr | (! grep -i 'ERROR in')
     - yarn run unit
   cache:
     key: "$CI_PROJECT_ID__front_dependencies"
@@ -70,7 +71,9 @@ build_front:
     - yarn install
     - yarn run i18n-extract
     - yarn run i18n-compile
-    - yarn run build
+    # this is to ensure we don't have any errors in the output,
+    # cf https://code.eliotberriot.com/funkwhale/funkwhale/issues/169
+    - yarn run build | tee /dev/stderr | (! grep -i 'ERROR in')
   cache:
     key: "$CI_PROJECT_ID__front_dependencies"
     paths:

From 74e7aed2d492fc6aacb38dcd4d6b2ee4c532040f Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Fri, 20 Apr 2018 19:49:56 +0200
Subject: [PATCH 11/23] Removed duplicated line

---
 .gitlab-ci.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index b2078f153..792c7e1b2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -46,7 +46,6 @@ test_front:
 
   script:
     - yarn install
-    - yarn run build | tee /dev/stderr | (! grep -i 'ERROR in')
     - yarn run unit
   cache:
     key: "$CI_PROJECT_ID__front_dependencies"

From e29cfb73019ada8e091739d33b1b43e10d560ebf Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 16:01:42 +0200
Subject: [PATCH 12/23] Smarted UUID validation on metadata

---
 api/funkwhale_api/music/metadata.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/api/funkwhale_api/music/metadata.py b/api/funkwhale_api/music/metadata.py
index 3748d5573..a20069783 100644
--- a/api/funkwhale_api/music/metadata.py
+++ b/api/funkwhale_api/music/metadata.py
@@ -1,5 +1,6 @@
-import mutagen
+from django import forms
 import arrow
+import mutagen
 
 NODEFAULT = object()
 
@@ -50,6 +51,13 @@ def convert_track_number(v):
     except (ValueError, AttributeError, IndexError):
         pass
 
+
+VALIDATION = {
+    'musicbrainz_artistid': forms.UUIDField(),
+    'musicbrainz_albumid': forms.UUIDField(),
+    'musicbrainz_recordingid': forms.UUIDField(),
+}
+
 CONF = {
     'OggVorbis': {
         'getter': lambda f, k: f[k][0],
@@ -146,4 +154,7 @@ class Metadata(object):
         converter = field_conf.get('to_application')
         if converter:
             v = converter(v)
+        field = VALIDATION.get(key)
+        if field:
+            v = field.to_python(v)
         return v

From d98c33e5eddce0a883e46e2eaabe7e381cb638be Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 16:02:11 +0200
Subject: [PATCH 13/23] More efficient SQL query to compute import batch status

---
 api/funkwhale_api/music/models.py | 2 ++
 api/funkwhale_api/music/utils.py  | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py
index cc3849d50..18f181e88 100644
--- a/api/funkwhale_api/music/models.py
+++ b/api/funkwhale_api/music/models.py
@@ -507,6 +507,8 @@ class ImportBatch(models.Model):
     def update_status(self):
         old_status = self.status
         self.status = utils.compute_status(self.jobs.all())
+        if self.status == old_status:
+            return
         self.save(update_fields=['status'])
         if self.status != old_status and self.status == 'finished':
             from . import tasks
diff --git a/api/funkwhale_api/music/utils.py b/api/funkwhale_api/music/utils.py
index af0e59ab4..95f9cc62f 100644
--- a/api/funkwhale_api/music/utils.py
+++ b/api/funkwhale_api/music/utils.py
@@ -53,10 +53,11 @@ def guess_mimetype(f):
 
 
 def compute_status(jobs):
-    errored = any([job.status == 'errored' for job in jobs])
+    statuses = jobs.values_list('status', flat=True).distinct()
+    errored = any([status == 'errored' for status in statuses])
     if errored:
         return 'errored'
-    pending = any([job.status == 'pending' for job in jobs])
+    pending = any([status == 'pending' for status in statuses])
     if pending:
         return 'pending'
     return 'finished'

From 1672230f39344363fe96c1b8dace749dbc079f14 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 16:02:42 +0200
Subject: [PATCH 14/23] More verbose/robust import_files command, also skips
 existing files

---
 api/funkwhale_api/music/utils.py              |  2 +-
 .../management/commands/import_files.py       | 67 ++++++++++++++-----
 api/tests/test_import_audio_file.py           | 16 +++++
 3 files changed, 68 insertions(+), 17 deletions(-)

diff --git a/api/funkwhale_api/music/utils.py b/api/funkwhale_api/music/utils.py
index 95f9cc62f..7a851f7cc 100644
--- a/api/funkwhale_api/music/utils.py
+++ b/api/funkwhale_api/music/utils.py
@@ -53,7 +53,7 @@ def guess_mimetype(f):
 
 
 def compute_status(jobs):
-    statuses = jobs.values_list('status', flat=True).distinct()
+    statuses = jobs.order_by().values_list('status', flat=True).distinct()
     errored = any([status == 'errored' for status in statuses])
     if errored:
         return 'errored'
diff --git a/api/funkwhale_api/providers/audiofile/management/commands/import_files.py b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py
index dbc01289f..0be73c2da 100644
--- a/api/funkwhale_api/providers/audiofile/management/commands/import_files.py
+++ b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py
@@ -3,9 +3,8 @@ import os
 
 from django.core.files import File
 from django.core.management.base import BaseCommand, CommandError
-from django.db import transaction
 
-from funkwhale_api.common import utils
+from funkwhale_api.music import models
 from funkwhale_api.music import tasks
 from funkwhale_api.users.models import User
 
@@ -66,9 +65,6 @@ class Command(BaseCommand):
         except TypeError:
             raise Exception('You need Python 3.5 to use the --recursive flag')
 
-        self.stdout.write('This will import {} files matching this pattern: {}'.format(
-            len(matching), options['path']))
-
         if not matching:
             raise CommandError('No file matching pattern, aborting')
 
@@ -86,6 +82,20 @@ class Command(BaseCommand):
             except AssertionError:
                 raise CommandError(
                     'No superuser available, please provide a --username')
+
+        filtered = self.filter_matching(matching, options)
+        self.stdout.write('Import summary:')
+        self.stdout.write('- {} files found matching this pattern: {}'.format(
+            len(matching), options['path']))
+        self.stdout.write('- {} files already found in database'.format(
+            len(filtered['skipped'])))
+        self.stdout.write('- {} new files'.format(
+            len(filtered['new'])))
+
+        if len(filtered['new']) == 0:
+            self.stdout.write('Nothing new to import, exiting')
+            return
+
         if options['interactive']:
             message = (
                 'Are you sure you want to do this?\n\n'
@@ -94,27 +104,52 @@ class Command(BaseCommand):
             if input(''.join(message)) != 'yes':
                 raise CommandError("Import cancelled.")
 
-        batch = self.do_import(matching, user=user, options=options)
+        batch, errors = self.do_import(
+            filtered['new'], user=user, options=options)
         message = 'Successfully imported {} tracks'
         if options['async']:
             message = 'Successfully launched import for {} tracks'
+
         self.stdout.write(message.format(len(matching)))
+        if len(errors) > 0:
+            self.stderr.write(
+                '{} tracks could not be imported:'.format(len(errors)))
+
+            for path, error in errors:
+                self.stderr('- {}: {}'.format(path, error))
         self.stdout.write(
             "For details, please refer to import batch #{}".format(batch.pk))
 
-    @transaction.atomic
-    def do_import(self, matching, user, options):
-        message = 'Importing {}...'
+    def filter_matching(self, matching, options):
+        sources = ['file://{}'.format(p) for p in matching]
+        # we skip reimport for path that are already found
+        # as a TrackFile.source
+        existing = models.TrackFile.objects.filter(source__in=sources)
+        existing = existing.values_list('source', flat=True)
+        existing = set([p.replace('file://', '', 1) for p in existing])
+        skipped = set(matching) & existing
+        result = {
+            'initial': matching,
+            'skipped': list(skipped),
+            'new': list(set(matching) - skipped)
+        }
+        return result
+
+    def do_import(self, paths, user, options):
+        message = '{i}/{total} Importing {path}...'
         if options['async']:
-            message = 'Launching import for {}...'
+            message = '{i}/{total} Launching import for {path}...'
 
         # we create an import batch binded to the user
-        batch = user.imports.create(source='shell')
         async = options['async']
         import_handler = tasks.import_job_run.delay if async else tasks.import_job_run
-        for path in matching:
+        batch = user.imports.create(source='shell')
+        total = len(paths)
+        errors = []
+        for i, path in enumerate(paths):
             try:
-                self.stdout.write(message.format(path))
+                self.stdout.write(
+                    message.format(path=path, i=i+1, total=len(paths)))
                 self.import_file(path, batch, import_handler, options)
             except Exception as e:
                 if options['exit_on_failure']:
@@ -122,7 +157,8 @@ class Command(BaseCommand):
                 m = 'Error while importing {}: {} {}'.format(
                     path, e.__class__.__name__, e)
                 self.stderr.write(m)
-        return batch
+                errors.append((m, path))
+        return batch, errors
 
     def import_file(self, path, batch, import_handler, options):
         job = batch.jobs.create(
@@ -133,7 +169,6 @@ class Command(BaseCommand):
             job.audio_file.save(name, File(f))
 
         job.save()
-        utils.on_commit(
-            import_handler,
+        import_handler(
             import_job_id=job.pk,
             use_acoustid=not options['no_acoustid'])
diff --git a/api/tests/test_import_audio_file.py b/api/tests/test_import_audio_file.py
index 67263e66d..f291b8133 100644
--- a/api/tests/test_import_audio_file.py
+++ b/api/tests/test_import_audio_file.py
@@ -100,6 +100,22 @@ def test_import_files_skip_acoustid(factories, mocker):
         use_acoustid=False)
 
 
+def test_import_files_skip_if_path_already_imported(factories, mocker):
+    user = factories['users.User'](username='me')
+    path = os.path.join(DATA_DIR, 'dummy_file.ogg')
+    existing = factories['music.TrackFile'](
+        source='file://{}'.format(path))
+
+    call_command(
+        'import_files',
+        path,
+        username='me',
+        async=False,
+        no_acoustid=True,
+        interactive=False)
+    assert user.imports.count() == 0
+
+
 def test_import_files_works_with_utf8_file_name(factories, mocker):
     m = mocker.patch('funkwhale_api.common.utils.on_commit')
     user = factories['users.User'](username='me')

From 3f3d6d88d64c428923ad0a6c9d7500ce619f0850 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 16:02:56 +0200
Subject: [PATCH 15/23] Make music directory configurable in development

---
 .../audiofile/management/commands/import_files.py  | 14 +++++++-------
 api/funkwhale_api/providers/audiofile/tasks.py     |  3 +++
 dev.yml                                            |  4 ++--
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/api/funkwhale_api/providers/audiofile/management/commands/import_files.py b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py
index 0be73c2da..fca98ccd3 100644
--- a/api/funkwhale_api/providers/audiofile/management/commands/import_files.py
+++ b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py
@@ -61,7 +61,7 @@ class Command(BaseCommand):
         if options['recursive']:
             glob_kwargs['recursive'] = True
         try:
-            matching = glob.glob(options['path'], **glob_kwargs)
+            matching = sorted(glob.glob(options['path'], **glob_kwargs))
         except TypeError:
             raise Exception('You need Python 3.5 to use the --recursive flag')
 
@@ -110,13 +110,13 @@ class Command(BaseCommand):
         if options['async']:
             message = 'Successfully launched import for {} tracks'
 
-        self.stdout.write(message.format(len(matching)))
+        self.stdout.write(message.format(len(filtered['new'])))
         if len(errors) > 0:
             self.stderr.write(
                 '{} tracks could not be imported:'.format(len(errors)))
 
             for path, error in errors:
-                self.stderr('- {}: {}'.format(path, error))
+                self.stderr.write('- {}: {}'.format(path, error))
         self.stdout.write(
             "For details, please refer to import batch #{}".format(batch.pk))
 
@@ -130,8 +130,8 @@ class Command(BaseCommand):
         skipped = set(matching) & existing
         result = {
             'initial': matching,
-            'skipped': list(skipped),
-            'new': list(set(matching) - skipped)
+            'skipped': list(sorted(skipped)),
+            'new': list(sorted(set(matching) - skipped)),
         }
         return result
 
@@ -146,7 +146,7 @@ class Command(BaseCommand):
         batch = user.imports.create(source='shell')
         total = len(paths)
         errors = []
-        for i, path in enumerate(paths):
+        for i, path in list(enumerate(paths)):
             try:
                 self.stdout.write(
                     message.format(path=path, i=i+1, total=len(paths)))
@@ -157,7 +157,7 @@ class Command(BaseCommand):
                 m = 'Error while importing {}: {} {}'.format(
                     path, e.__class__.__name__, e)
                 self.stderr.write(m)
-                errors.append((m, path))
+                errors.append((path, '{} {}'.format(e.__class__.__name__, e)))
         return batch, errors
 
     def import_file(self, path, batch, import_handler, options):
diff --git a/api/funkwhale_api/providers/audiofile/tasks.py b/api/funkwhale_api/providers/audiofile/tasks.py
index bc18456c3..40114c877 100644
--- a/api/funkwhale_api/providers/audiofile/tasks.py
+++ b/api/funkwhale_api/providers/audiofile/tasks.py
@@ -2,12 +2,14 @@ import acoustid
 import os
 import datetime
 from django.core.files import File
+from django.db import transaction
 
 from funkwhale_api.taskapp import celery
 from funkwhale_api.providers.acoustid import get_acoustid_client
 from funkwhale_api.music import models, metadata
 
 
+@transaction.atomic
 def import_track_data_from_path(path):
     data = metadata.Metadata(path)
     artist = models.Artist.objects.get_or_create(
@@ -45,6 +47,7 @@ def import_track_data_from_path(path):
 def import_metadata_with_musicbrainz(path):
     pass
 
+
 @celery.app.task(name='audiofile.from_path')
 def from_path(path):
     acoustid_track_id = None
diff --git a/dev.yml b/dev.yml
index 2df7b44e6..3f67af798 100644
--- a/dev.yml
+++ b/dev.yml
@@ -65,7 +65,7 @@ services:
       - "CACHE_URL=redis://redis:6379/0"
     volumes:
       - ./api:/app
-      - ./data/music:/music
+      - "${MUSIC_DIRECTORY-./data/music}:/music"
     networks:
       - internal
   api:
@@ -78,7 +78,7 @@ services:
     command: python /app/manage.py runserver 0.0.0.0:12081
     volumes:
       - ./api:/app
-      - ./data/music:/music
+      - "${MUSIC_DIRECTORY-./data/music}:/music"
     environment:
       - "FUNKWHALE_HOSTNAME=${FUNKWHALE_HOSTNAME-localhost}"
       - "FUNKWHALE_HOSTNAME_SUFFIX=funkwhale.test"

From 6395f48b2b35fb189c155b8db041ccbbd7b5a8d0 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 16:16:09 +0200
Subject: [PATCH 16/23] Fix #144: changelog

---
 changes/changelog.d/144.enhancement | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changes/changelog.d/144.enhancement

diff --git a/changes/changelog.d/144.enhancement b/changes/changelog.d/144.enhancement
new file mode 100644
index 000000000..2c238ed21
--- /dev/null
+++ b/changes/changelog.d/144.enhancement
@@ -0,0 +1 @@
+Better file import performance and error handling (#144)

From 0d3eb96becb418de77120b748b778b045a303542 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 16:33:15 +0200
Subject: [PATCH 17/23] Fixed failing tests

---
 api/tests/music/test_metadata.py    | 13 +++++++------
 api/tests/test_import_audio_file.py | 30 +++++++++++++++--------------
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/api/tests/music/test_metadata.py b/api/tests/music/test_metadata.py
index 5df2dbcf1..342bc99b8 100644
--- a/api/tests/music/test_metadata.py
+++ b/api/tests/music/test_metadata.py
@@ -1,6 +1,7 @@
 import datetime
 import os
 import pytest
+import uuid
 
 from funkwhale_api.music import metadata
 
@@ -13,9 +14,9 @@ DATA_DIR = os.path.dirname(os.path.abspath(__file__))
     ('album', 'Peer Gynt Suite no. 1, op. 46'),
     ('date', datetime.date(2012, 8, 15)),
     ('track_number', 1),
-    ('musicbrainz_albumid', 'a766da8b-8336-47aa-a3ee-371cc41ccc75'),
-    ('musicbrainz_recordingid', 'bd21ac48-46d8-4e78-925f-d9cc2a294656'),
-    ('musicbrainz_artistid', '013c8e5b-d72a-4cd3-8dee-6c64d6125823'),
+    ('musicbrainz_albumid', uuid.UUID('a766da8b-8336-47aa-a3ee-371cc41ccc75')),
+    ('musicbrainz_recordingid', uuid.UUID('bd21ac48-46d8-4e78-925f-d9cc2a294656')),
+    ('musicbrainz_artistid', uuid.UUID('013c8e5b-d72a-4cd3-8dee-6c64d6125823')),
 ])
 def test_can_get_metadata_from_ogg_file(field, value):
     path = os.path.join(DATA_DIR, 'test.ogg')
@@ -30,9 +31,9 @@ def test_can_get_metadata_from_ogg_file(field, value):
     ('album', 'You Can\'t Stop Da Funk'),
     ('date', datetime.date(2006, 2, 7)),
     ('track_number', 1),
-    ('musicbrainz_albumid', 'ce40cdb1-a562-4fd8-a269-9269f98d4124'),
-    ('musicbrainz_recordingid', 'f269d497-1cc0-4ae4-a0c4-157ec7d73fcb'),
-    ('musicbrainz_artistid', '9c6bddde-6228-4d9f-ad0d-03f6fcb19e13'),
+    ('musicbrainz_albumid', uuid.UUID('ce40cdb1-a562-4fd8-a269-9269f98d4124')),
+    ('musicbrainz_recordingid', uuid.UUID('f269d497-1cc0-4ae4-a0c4-157ec7d73fcb')),
+    ('musicbrainz_artistid', uuid.UUID('9c6bddde-6228-4d9f-ad0d-03f6fcb19e13')),
 ])
 def test_can_get_metadata_from_id3_mp3_file(field, value):
     path = os.path.join(DATA_DIR, 'test.mp3')
diff --git a/api/tests/test_import_audio_file.py b/api/tests/test_import_audio_file.py
index f291b8133..0fe484fa5 100644
--- a/api/tests/test_import_audio_file.py
+++ b/api/tests/test_import_audio_file.py
@@ -2,6 +2,8 @@ import pytest
 import acoustid
 import datetime
 import os
+import uuid
+
 from django.core.management import call_command
 from django.core.management.base import CommandError
 
@@ -15,7 +17,8 @@ DATA_DIR = os.path.join(
 
 
 def test_can_create_track_from_file_metadata(db, mocker):
-    mocker.patch('acoustid.match', side_effect=acoustid.WebServiceError('test'))
+    mocker.patch(
+        'acoustid.match', side_effect=acoustid.WebServiceError('test'))
     metadata = {
         'artist': ['Test artist'],
         'album': ['Test album'],
@@ -35,33 +38,35 @@ def test_can_create_track_from_file_metadata(db, mocker):
         os.path.join(DATA_DIR, 'dummy_file.ogg'))
 
     assert track.title == metadata['title'][0]
-    assert track.mbid == metadata['musicbrainz_trackid'][0]
+    assert track.mbid == uuid.UUID(metadata['musicbrainz_trackid'][0])
     assert track.position == 4
     assert track.album.title == metadata['album'][0]
-    assert track.album.mbid == metadata['musicbrainz_albumid'][0]
+    assert track.album.mbid == uuid.UUID(metadata['musicbrainz_albumid'][0])
     assert track.album.release_date == datetime.date(2012, 8, 15)
     assert track.artist.name == metadata['artist'][0]
-    assert track.artist.mbid == metadata['musicbrainz_artistid'][0]
+    assert track.artist.mbid == uuid.UUID(metadata['musicbrainz_artistid'][0])
 
 
 def test_management_command_requires_a_valid_username(factories, mocker):
     path = os.path.join(DATA_DIR, 'dummy_file.ogg')
     user = factories['users.User'](username='me')
-    mocker.patch('funkwhale_api.providers.audiofile.management.commands.import_files.Command.do_import')  # NOQA
+    mocker.patch(
+        'funkwhale_api.providers.audiofile.management.commands.import_files.Command.do_import',  # noqa
+        return_value=(mocker.MagicMock(), []))
     with pytest.raises(CommandError):
         call_command('import_files', path, username='not_me', interactive=False)
     call_command('import_files', path, username='me', interactive=False)
 
 
 def test_import_files_creates_a_batch_and_job(factories, mocker):
-    m = mocker.patch('funkwhale_api.common.utils.on_commit')
+    m = mocker.patch('funkwhale_api.music.tasks.import_job_run')
     user = factories['users.User'](username='me')
     path = os.path.join(DATA_DIR, 'dummy_file.ogg')
     call_command(
         'import_files',
         path,
         username='me',
-        async=True,
+        async=False,
         interactive=False)
 
     batch = user.imports.latest('id')
@@ -76,26 +81,24 @@ def test_import_files_creates_a_batch_and_job(factories, mocker):
 
     assert job.source == 'file://' + path
     m.assert_called_once_with(
-        music_tasks.import_job_run.delay,
         import_job_id=job.pk,
         use_acoustid=True)
 
 
 def test_import_files_skip_acoustid(factories, mocker):
-    m = mocker.patch('funkwhale_api.common.utils.on_commit')
+    m = mocker.patch('funkwhale_api.music.tasks.import_job_run')
     user = factories['users.User'](username='me')
     path = os.path.join(DATA_DIR, 'dummy_file.ogg')
     call_command(
         'import_files',
         path,
         username='me',
-        async=True,
+        async=False,
         no_acoustid=True,
         interactive=False)
     batch = user.imports.latest('id')
     job = batch.jobs.first()
     m.assert_called_once_with(
-        music_tasks.import_job_run.delay,
         import_job_id=job.pk,
         use_acoustid=False)
 
@@ -117,20 +120,19 @@ def test_import_files_skip_if_path_already_imported(factories, mocker):
 
 
 def test_import_files_works_with_utf8_file_name(factories, mocker):
-    m = mocker.patch('funkwhale_api.common.utils.on_commit')
+    m = mocker.patch('funkwhale_api.music.tasks.import_job_run')
     user = factories['users.User'](username='me')
     path = os.path.join(DATA_DIR, 'utf8-éà◌.ogg')
     call_command(
         'import_files',
         path,
         username='me',
-        async=True,
+        async=False,
         no_acoustid=True,
         interactive=False)
     batch = user.imports.latest('id')
     job = batch.jobs.first()
     m.assert_called_once_with(
-        music_tasks.import_job_run.delay,
         import_job_id=job.pk,
         use_acoustid=False)
 

From a8bf44a49434c95fd59932e289538baed0c6617f Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 18:15:29 +0200
Subject: [PATCH 18/23] Added dedicated settings to manage in-place import

---
 .env.dev                      | 1 +
 api/config/settings/common.py | 6 ++++++
 dev.yml                       | 5 +++--
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/.env.dev b/.env.dev
index 3f904078c..c09262509 100644
--- a/.env.dev
+++ b/.env.dev
@@ -9,3 +9,4 @@ FUNKWHALE_HOSTNAME=localhost
 FUNKWHALE_PROTOCOL=http
 PYTHONDONTWRITEBYTECODE=true
 WEBPACK_DEVSERVER_PORT=8080
+MUSIC_DIRECTORY_PATH=/music
diff --git a/api/config/settings/common.py b/api/config/settings/common.py
index a972ec7ef..5e895bea5 100644
--- a/api/config/settings/common.py
+++ b/api/config/settings/common.py
@@ -441,3 +441,9 @@ EXTERNAL_REQUESTS_VERIFY_SSL = env.bool(
     'EXTERNAL_REQUESTS_VERIFY_SSL',
     default=True
 )
+
+MUSIC_DIRECTORY_PATH = env('MUSIC_DIRECTORY_PATH', default=None)
+# on Docker setup, the music directory may not match the host path,
+# and we need to know it for it to serve stuff properly
+MUSIC_DIRECTORY_SERVE_PATH = env(
+    'MUSIC_DIRECTORY_SERVE_PATH', default=MUSIC_DIRECTORY_PATH)
diff --git a/dev.yml b/dev.yml
index 3f67af798..264fc9534 100644
--- a/dev.yml
+++ b/dev.yml
@@ -65,7 +65,7 @@ services:
       - "CACHE_URL=redis://redis:6379/0"
     volumes:
       - ./api:/app
-      - "${MUSIC_DIRECTORY-./data/music}:/music"
+      - "${MUSIC_DIRECTORY-./data/music}:/music:ro"
     networks:
       - internal
   api:
@@ -78,7 +78,7 @@ services:
     command: python /app/manage.py runserver 0.0.0.0:12081
     volumes:
       - ./api:/app
-      - "${MUSIC_DIRECTORY-./data/music}:/music"
+      - "${MUSIC_DIRECTORY-./data/music}:/music:ro"
     environment:
       - "FUNKWHALE_HOSTNAME=${FUNKWHALE_HOSTNAME-localhost}"
       - "FUNKWHALE_HOSTNAME_SUFFIX=funkwhale.test"
@@ -107,6 +107,7 @@ services:
     volumes:
       - ./docker/nginx/conf.dev:/etc/nginx/nginx.conf
       - ./docker/nginx/entrypoint.sh:/entrypoint.sh:ro
+      - "${MUSIC_DIRECTORY-./data/music}:/music:ro"
       - ./deploy/funkwhale_proxy.conf:/etc/nginx/funkwhale_proxy.conf.template:ro
       - ./api/funkwhale_api/media:/protected/media
     ports:

From de754b835e12fea863c79f2c0980f5d53ba9b813 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 18:16:43 +0200
Subject: [PATCH 19/23] Updated CLI to support in-place import

---
 .../management/commands/import_files.py       | 48 +++++++++++++++----
 api/tests/test_import_audio_file.py           | 35 ++++++++++++++
 2 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/api/funkwhale_api/providers/audiofile/management/commands/import_files.py b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py
index fca98ccd3..a2757c692 100644
--- a/api/funkwhale_api/providers/audiofile/management/commands/import_files.py
+++ b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py
@@ -1,6 +1,7 @@
 import glob
 import os
 
+from django.conf import settings
 from django.core.files import File
 from django.core.management.base import BaseCommand, CommandError
 
@@ -38,7 +39,20 @@ class Command(BaseCommand):
             action='store_true',
             dest='exit_on_failure',
             default=False,
-            help='use this flag to disable error catching',
+            help='Use this flag to disable error catching',
+        )
+        parser.add_argument(
+            '--in-place', '-i',
+            action='store_true',
+            dest='in_place',
+            default=False,
+            help=(
+                'Import files without duplicating them into the media directory.'
+                'For in-place import to work, the music files must be readable'
+                'by the web-server and funkwhale api and celeryworker processes.'
+                'You may want to use this if you have a big music library to '
+                'import and not much disk space available.'
+            )
         )
         parser.add_argument(
             '--no-acoustid',
@@ -53,10 +67,6 @@ class Command(BaseCommand):
         )
 
     def handle(self, *args, **options):
-        # self.stdout.write(self.style.SUCCESS('Successfully closed poll "%s"' % poll_id))
-
-        # Recursive is supported only on Python 3.5+, so we pass the option
-        # only if it's True to avoid breaking on older versions of Python
         glob_kwargs = {}
         if options['recursive']:
             glob_kwargs['recursive'] = True
@@ -65,6 +75,21 @@ class Command(BaseCommand):
         except TypeError:
             raise Exception('You need Python 3.5 to use the --recursive flag')
 
+        if options['in_place']:
+            self.stdout.write(
+                'Checking imported paths against settings.MUSIC_DIRECTORY_PATH')
+            p = settings.MUSIC_DIRECTORY_PATH
+            if not p:
+                raise CommandError(
+                    'Importing in-place requires setting the '
+                    'MUSIC_DIRECTORY_PATH variable')
+            for m in matching:
+                if not m.startswith(p):
+                    raise CommandError(
+                        'Importing in-place only works if importing'
+                        'from {} (MUSIC_DIRECTORY_PATH), as this directory'
+                        'needs to be accessible by the webserver.'
+                        'Culprit: {}'.format(p, m))
         if not matching:
             raise CommandError('No file matching pattern, aborting')
 
@@ -92,6 +117,10 @@ class Command(BaseCommand):
         self.stdout.write('- {} new files'.format(
             len(filtered['new'])))
 
+        self.stdout.write('Selected options: {}'.format(', '.join([
+            'no acoustid' if options['no_acoustid'] else 'use acoustid',
+            'in place' if options['in_place'] else 'copy music files',
+        ])))
         if len(filtered['new']) == 0:
             self.stdout.write('Nothing new to import, exiting')
             return
@@ -164,11 +193,12 @@ class Command(BaseCommand):
         job = batch.jobs.create(
             source='file://' + path,
         )
-        name = os.path.basename(path)
-        with open(path, 'rb') as f:
-            job.audio_file.save(name, File(f))
+        if not options['in_place']:
+            name = os.path.basename(path)
+            with open(path, 'rb') as f:
+                job.audio_file.save(name, File(f))
 
-        job.save()
+            job.save()
         import_handler(
             import_job_id=job.pk,
             use_acoustid=not options['no_acoustid'])
diff --git a/api/tests/test_import_audio_file.py b/api/tests/test_import_audio_file.py
index 0fe484fa5..8217ffa0b 100644
--- a/api/tests/test_import_audio_file.py
+++ b/api/tests/test_import_audio_file.py
@@ -58,6 +58,20 @@ def test_management_command_requires_a_valid_username(factories, mocker):
     call_command('import_files', path, username='me', interactive=False)
 
 
+def test_in_place_import_only_from_music_dir(factories, settings):
+    user = factories['users.User'](username='me')
+    settings.MUSIC_DIRECTORY_PATH = '/nope'
+    path = os.path.join(DATA_DIR, 'dummy_file.ogg')
+    with pytest.raises(CommandError):
+        call_command(
+            'import_files',
+            path,
+            in_place=True,
+            username='me',
+            interactive=False
+        )
+
+
 def test_import_files_creates_a_batch_and_job(factories, mocker):
     m = mocker.patch('funkwhale_api.music.tasks.import_job_run')
     user = factories['users.User'](username='me')
@@ -137,6 +151,27 @@ def test_import_files_works_with_utf8_file_name(factories, mocker):
         use_acoustid=False)
 
 
+def test_import_files_in_place(factories, mocker, settings):
+    settings.MUSIC_DIRECTORY_PATH = DATA_DIR
+    m = mocker.patch('funkwhale_api.music.tasks.import_job_run')
+    user = factories['users.User'](username='me')
+    path = os.path.join(DATA_DIR, 'utf8-éà◌.ogg')
+    call_command(
+        'import_files',
+        path,
+        username='me',
+        async=False,
+        in_place=True,
+        no_acoustid=True,
+        interactive=False)
+    batch = user.imports.latest('id')
+    job = batch.jobs.first()
+    assert bool(job.audio_file) is False
+    m.assert_called_once_with(
+        import_job_id=job.pk,
+        use_acoustid=False)
+
+
 def test_storage_rename_utf_8_files(factories):
     tf = factories['music.TrackFile'](audio_file__filename='été.ogg')
     assert tf.audio_file.name.endswith('ete.ogg')

From 58fced2640ef8bad038cd856e3fd3f1b557f5e73 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 18:20:56 +0200
Subject: [PATCH 20/23] updated import_task to support in place import

---
 api/funkwhale_api/music/factories.py |  8 ++++++++
 api/funkwhale_api/music/tasks.py     | 12 ++++++++----
 api/tests/music/test_import.py       | 12 ++++++++++++
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py
index ea7ff64df..bc0c74a2d 100644
--- a/api/funkwhale_api/music/factories.py
+++ b/api/funkwhale_api/music/factories.py
@@ -43,6 +43,7 @@ class TrackFactory(factory.django.DjangoModelFactory):
     artist = factory.SelfAttribute('album.artist')
     position = 1
     tags = ManyToManyFromList('tags')
+
     class Meta:
         model = 'music.Track'
 
@@ -57,6 +58,9 @@ class TrackFileFactory(factory.django.DjangoModelFactory):
         model = 'music.TrackFile'
 
     class Params:
+        in_place = factory.Trait(
+            audio_file=None,
+        )
         federation = factory.Trait(
             audio_file=None,
             library_track=factory.SubFactory(LibraryTrackFactory),
@@ -105,6 +109,10 @@ class ImportJobFactory(factory.django.DjangoModelFactory):
             status='finished',
             track_file=factory.SubFactory(TrackFileFactory),
         )
+        in_place = factory.Trait(
+            status='finished',
+            audio_file=None,
+        )
 
 
 @registry.register(name='music.FileImportJob')
diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py
index bc5ab94f0..4b9e15fc9 100644
--- a/api/funkwhale_api/music/tasks.py
+++ b/api/funkwhale_api/music/tasks.py
@@ -71,7 +71,7 @@ def import_track_from_remote(library_track):
         library_track.title, artist=artist, album=album)
 
 
-def _do_import(import_job, replace, use_acoustid=True):
+def _do_import(import_job, replace=False, use_acoustid=True):
     from_file = bool(import_job.audio_file)
     mbid = import_job.mbid
     acoustid_track_id = None
@@ -93,6 +93,9 @@ def _do_import(import_job, replace, use_acoustid=True):
         track = import_track_data_from_path(import_job.audio_file.path)
     elif import_job.library_track:
         track = import_track_from_remote(import_job.library_track)
+    elif import_job.source.startswith('file://'):
+        track = import_track_data_from_path(
+            import_job.source.replace('file://', '', 1))
     else:
         raise ValueError(
             'Not enough data to process import, '
@@ -123,7 +126,7 @@ def _do_import(import_job, replace, use_acoustid=True):
         else:
             # no downloading, we hotlink
             pass
-    else:
+    elif import_job.audio_file:
         track_file.download_file()
     track_file.save()
     import_job.status = 'finished'
@@ -133,7 +136,7 @@ def _do_import(import_job, replace, use_acoustid=True):
         import_job.audio_file.delete()
     import_job.save()
 
-    return track.pk
+    return track_file
 
 
 @celery.app.task(name='ImportJob.run', bind=True)
@@ -147,7 +150,8 @@ def import_job_run(self, import_job, replace=False, use_acoustid=True):
         import_job.save(update_fields=['status'])
 
     try:
-        return _do_import(import_job, replace, use_acoustid=use_acoustid)
+        tf = _do_import(import_job, replace, use_acoustid=use_acoustid)
+        return tf.pk if tf else None
     except Exception as exc:
         if not settings.DEBUG:
             try:
diff --git a/api/tests/music/test_import.py b/api/tests/music/test_import.py
index 2f22ed69a..65e0242fb 100644
--- a/api/tests/music/test_import.py
+++ b/api/tests/music/test_import.py
@@ -231,3 +231,15 @@ def test_import_batch_notifies_followers(
         on_behalf_of=library_actor,
         to=[f1.actor.url]
     )
+
+
+def test__do_import_in_place_mbid(factories, tmpfile):
+    path = '/test.ogg'
+    job = factories['music.ImportJob'](
+        in_place=True, source='file:///test.ogg')
+
+    track = factories['music.Track'](mbid=job.mbid)
+    tf = tasks._do_import(job, use_acoustid=False)
+
+    assert bool(tf.audio_file) is False
+    assert tf.source == 'file:///test.ogg'

From f4ac0b2d3fc0f3ea665336ad010d25580b93b694 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 18:21:15 +0200
Subject: [PATCH 21/23] Serve view can now serve in-place imported files

---
 api/funkwhale_api/music/models.py | 20 ++++++++++++++++++++
 api/funkwhale_api/music/views.py  | 15 ++++++++++++---
 api/tests/music/test_views.py     | 19 +++++++++++++++++++
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py
index 18f181e88..98fc1965b 100644
--- a/api/funkwhale_api/music/models.py
+++ b/api/funkwhale_api/music/models.py
@@ -463,6 +463,26 @@ class TrackFile(models.Model):
             self.mimetype = utils.guess_mimetype(self.audio_file)
         return super().save(**kwargs)
 
+    @property
+    def serve_from_source_path(self):
+        if not self.source or not self.source.startswith('file://'):
+            raise ValueError('Cannot serve this file from source')
+        serve_path = settings.MUSIC_DIRECTORY_SERVE_PATH
+        prefix = settings.MUSIC_DIRECTORY_PATH
+        if not serve_path or not prefix:
+            raise ValueError(
+                'You need to specify MUSIC_DIRECTORY_SERVE_PATH and '
+                'MUSIC_DIRECTORY_PATH to serve in-place imported files'
+            )
+        file_path = self.source.replace('file://', '', 1)
+        parts = os.path.split(file_path.replace(prefix, '', 1))
+        if parts[0] == '/':
+            parts = parts[1:]
+        return os.path.join(
+            serve_path,
+            *parts
+        )
+
 
 IMPORT_STATUS_CHOICES = (
     ('pending', 'Pending'),
diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py
index f961e3fdf..d03b55e50 100644
--- a/api/funkwhale_api/music/views.py
+++ b/api/funkwhale_api/music/views.py
@@ -224,12 +224,21 @@ class TrackFileViewSet(viewsets.ReadOnlyModelViewSet):
                     library_track = qs.get(pk=library_track.pk)
                     library_track.download_audio()
             audio_file = library_track.audio_file
+            file_path = '{}{}'.format(
+                settings.PROTECT_FILES_PATH,
+                audio_file.url)
             mt = library_track.audio_mimetype
+        elif audio_file:
+            file_path = '{}{}'.format(
+                settings.PROTECT_FILES_PATH,
+                audio_file.url)
+        elif f.source and f.source.startswith('file://'):
+            file_path = '{}{}'.format(
+                settings.PROTECT_FILES_PATH + '/music',
+                f.serve_from_source_path)
         response = Response()
         filename = f.filename
-        response['X-Accel-Redirect'] = "{}{}".format(
-            settings.PROTECT_FILES_PATH,
-            audio_file.url)
+        response['X-Accel-Redirect'] = file_path
         filename = "filename*=UTF-8''{}".format(
             urllib.parse.quote(filename))
         response["Content-Disposition"] = "attachment; {}".format(filename)
diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py
index 81f34fbe9..95c56d914 100644
--- a/api/tests/music/test_views.py
+++ b/api/tests/music/test_views.py
@@ -93,6 +93,25 @@ def test_can_proxy_remote_track(
     assert library_track.audio_file.read() == b'test'
 
 
+def test_can_serve_in_place_imported_file(
+        factories, settings, api_client, r_mock):
+    settings.PROTECT_AUDIO_FILES = False
+    settings.MUSIC_DIRECTORY_SERVE_PATH = '/host/music'
+    settings.MUSIC_DIRECTORY_PATH = '/music'
+    settings.MUSIC_DIRECTORY_PATH = '/music'
+    track_file = factories['music.TrackFile'](
+        in_place=True,
+        source='file:///music/test.ogg')
+
+    response = api_client.get(track_file.path)
+
+    assert response.status_code == 200
+    assert response['X-Accel-Redirect'] == '{}{}'.format(
+        settings.PROTECT_FILES_PATH,
+        '/music/host/music/test.ogg'
+    )
+
+
 def test_can_create_import_from_federation_tracks(
         factories, superuser_api_client, mocker):
     lts = factories['federation.LibraryTrack'].create_batch(size=5)

From 70d3385aa37a6cfe4c84e3712434ca31754ce325 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 19:30:55 +0200
Subject: [PATCH 22/23] Documentation update for the new in-place import

---
 CHANGELOG                | 46 ++++++++++++++++++++++++++++++++++++++
 deploy/env.prod.sample   | 20 ++++++++++++++---
 deploy/nginx.conf        |  8 +++++++
 docker/nginx/conf.dev    |  4 ++++
 docs/configuration.rst   | 41 ++++++++++++++++++++++++++++++++++
 docs/importing-music.rst | 48 +++++++++++++++++++++++++++++++---------
 6 files changed, 154 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index b230b1556..c2d9be1a7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,6 +3,52 @@ Changelog
 
 .. towncrier
 
+0.10 (Unreleased)
+-----------------
+
+
+In-place import
+^^^^^^^^^^^^^^^
+
+This release includes in-place imports for the CLI import. This means you can
+load gigabytes of music into funkwhale without worrying about about Funkwhale
+copying those music files in its internal storage and eating your disk space.
+
+This new feature is documented <here> and require additional configuration
+to ensure funkwhale and your webserver can serve those files properly.
+
+**Non-docker users:**
+
+Assuming your music is stored in ``/srv/funkwhale/data/music``, add the following
+block to your nginx configuration::
+
+    location /_protected/music {
+        internal;
+        alias   /srv/funkwhale/data/music;
+    }
+
+And the following to your .env file::
+
+    MUSIC_DIRECTORY_PATH=/srv/funkwhale/data/music
+
+**Docker users:**
+
+Assuming your music is stored in ``/srv/funkwhale/data/music``, add the following
+block to your nginx configuration::
+
+    location /_protected/music {
+        internal;
+        alias   /srv/funkwhale/data/music;
+    }
+
+Assuming you have the following volume directive in your ``docker-compose.yml``
+(it's the default): ``/srv/funkwhale/data/music:/music:ro``, then add
+the following to your .env file::
+
+    MUSIC_DIRECTORY_PATH=/music
+    MUSIC_DIRECTORY_SERVE_PATH=/srv/funkwhale/data/music
+
+
 0.9.1 (2018-04-17)
 ------------------
 
diff --git a/deploy/env.prod.sample b/deploy/env.prod.sample
index 9e9938500..f33b06876 100644
--- a/deploy/env.prod.sample
+++ b/deploy/env.prod.sample
@@ -1,17 +1,22 @@
+# If you have any doubts about what a setting does,
+# check https://docs.funkwhale.audio/configuration.html#configuration-reference
+
 # If you're tweaking this file from the template, ensure you edit at least the
 # following variables:
 # - DJANGO_SECRET_KEY
 # - DJANGO_ALLOWED_HOSTS
 # - FUNKWHALE_URL
-
-# Additionaly, on non-docker setup **only**, you'll also have to tweak/uncomment those variables:
+# On non-docker setup **only**, you'll also have to tweak/uncomment those variables:
 # - DATABASE_URL
 # - CACHE_URL
 # - STATIC_ROOT
 # - MEDIA_ROOT
 #
 # You **don't** need to update those variables on pure docker setups.
-
+#
+# Additional options you may want to check:
+# - MUSIC_DIRECTORY_PATH and MUSIC_DIRECTORY_SERVE_PATH if you plan to use
+#   in-place import
 # Docker only
 # -----------
 
@@ -19,7 +24,9 @@
 # (it will be interpolated in docker-compose file)
 # You can comment or ignore this if you're not using docker
 FUNKWHALE_VERSION=latest
+MUSIC_DIRECTORY_PATH=/music
 
+# End of Docker-only configuration
 
 # General configuration
 # ---------------------
@@ -34,6 +41,7 @@ FUNKWHALE_API_PORT=5000
 # your instance
 FUNKWHALE_URL=https://yourdomain.funwhale
 
+
 # API/Django configuration
 
 # Database configuration
@@ -94,3 +102,9 @@ FEDERATION_ENABLED=True
 # means anyone can subscribe to your library and import your file,
 # use with caution.
 FEDERATION_MUSIC_NEEDS_APPROVAL=True
+
+# In-place import settings
+# You can safely leave those settings uncommented if you don't plan to use
+# in place imports.
+# MUSIC_DIRECTORY_PATH=
+# MUSIC_DIRECTORY_SERVE_PATH=
diff --git a/deploy/nginx.conf b/deploy/nginx.conf
index 1c304b493..b3a4c6aaf 100644
--- a/deploy/nginx.conf
+++ b/deploy/nginx.conf
@@ -84,6 +84,14 @@ server {
         alias   /srv/funkwhale/data/media;
     }
 
+    location /_protected/music {
+        # this is an internal location that is used to serve
+        # audio files once correct permission / authentication
+        # has been checked on API side
+        internal;
+        alias   /srv/funkwhale/data/music;
+    }
+
     # Transcoding logic and caching
     location = /transcode-auth {
         include /etc/nginx/funkwhale_proxy.conf;
diff --git a/docker/nginx/conf.dev b/docker/nginx/conf.dev
index e832a5ae3..38c3de6c7 100644
--- a/docker/nginx/conf.dev
+++ b/docker/nginx/conf.dev
@@ -42,6 +42,10 @@ http {
             internal;
             alias   /protected/media;
         }
+        location /_protected/music {
+            internal;
+            alias   /music;
+        }
         location = /transcode-auth {
             # needed so we can authenticate transcode requests, but still
             # cache the result
diff --git a/docs/configuration.rst b/docs/configuration.rst
index 5883a2d17..c0de76f56 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -33,3 +33,44 @@ The URL should be ``/api/admin/dynamic_preferences/globalpreferencemodel/`` (pre
 If you plan to use acoustid and external imports
 (e.g. with the youtube backends), you should edit the corresponding
 settings in this interface.
+
+Configuration reference
+-----------------------
+
+.. _setting-MUSIC_DIRECTORY_PATH:
+
+``MUSIC_DIRECTORY_PATH``
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+Default: ``None``
+
+The path on your server where Funwkhale can import files using :ref:`in-place import
+<in-place-import>`. It must be readable by the webserver and funkwhale
+api and worker processes.
+
+On docker installations, we recommend you use the default of ``/music``
+for this value. For non-docker installation, you can use any absolute path.
+``/srv/funkwhale/data/music`` is a safe choice if you don't know what to use.
+
+.. note:: This path should not include any trailing slash
+
+.. _setting-MUSIC_DIRECTORY_SERVE_PATH:
+
+``MUSIC_DIRECTORY_SERVE_PATH``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Default: :ref:`setting-MUSIC_DIRECTORY_PATH`
+
+When using Docker, the value of :ref:`MUSIC_DIRECTORY_PATH` in your containers
+may differ from the real path on your host. Assuming you have the following directive
+in your :file:`docker-compose.yml` file::
+
+    volumes:
+      - /srv/funkwhale/data/music:/music:ro
+
+Then, the value of :ref:`setting-MUSIC_DIRECTORY_SERVE_PATH` should be
+``/srv/funkwhale/data``. This must be readable by the webserver.
+
+On non-docker setup, you don't need to configure this setting.
+
+.. note:: This path should not include any trailing slash
diff --git a/docs/importing-music.rst b/docs/importing-music.rst
index f09eea7b1..97dd13854 100644
--- a/docs/importing-music.rst
+++ b/docs/importing-music.rst
@@ -22,8 +22,15 @@ to the ``/music`` directory on the container:
 
     docker-compose run --rm api python manage.py import_files "/music/**/*.ogg" --recursive --noinput
 
-For the best results, we recommand tagging your music collection through
-`Picard <http://picard.musicbrainz.org/>`_ in order to have the best quality metadata.
+The import command supports several options, and you can check the help to
+get details::
+
+    docker-compose run --rm api python manage.py import_files --help
+
+.. note::
+
+    For the best results, we recommand tagging your music collection through
+    `Picard <http://picard.musicbrainz.org/>`_ in order to have the best quality metadata.
 
 .. note::
 
@@ -39,18 +46,39 @@ For the best results, we recommand tagging your music collection through
 
     At the moment, only OGG/Vorbis and MP3 files with ID3 tags are supported
 
-.. note::
 
-    The --recursive flag will work only on Python 3.5+, which is the default
-    version When using Docker or Debian 9. If you use an older version of Python,
-    remove the --recursive flag and use more explicit import patterns instead::
+.. _in-place-import:
 
-        # this will only import ogg files at the second level
-        "/srv/funkwhale/data/music/*/*.ogg"
-        # this will only import ogg files in the fiven directory
-        "/srv/funkwhale/data/music/System-of-a-down/*.ogg"
+In-place import
+^^^^^^^^^^^^^^^
 
+By default, the CLI-importer will copy imported files to Funkwhale's internal
+storage. This means importing a 1Gb library will result in the same amount
+of space being used by Funkwhale.
 
+While this behaviour has some benefits (easier backups and configuration),
+it's not always the best choice, especially if you have a huge library
+to import and don't want to double your disk usage.
+
+The CLI importer supports an additional ``--in-place`` option that triggers the
+following behaviour during import:
+
+1. Imported files are not store in funkwhale anymore
+2. Instead, Funkwhale will store the file path and use it to serve the music
+
+Because those files are not managed by Funkwhale, we offer additional
+configuration options to ensure the webserver can serve them properly:
+
+- :ref:`setting-MUSIC_DIRECTORY_PATH`
+- :ref:`setting-MUSIC_DIRECTORY_SERVE_PATH`
+
+.. warning::
+
+    While in-place import is faster and less disk-space-hungry, it's also
+    more fragile: if, for some reason, you move or rename the source files,
+    Funkwhale will not be able to serve those files anymore.
+
+    Thus, be especially careful when you manipulate the source files.
 
 Getting demo tracks
 ^^^^^^^^^^^^^^^^^^^

From fb3361ce704bfb4f84b965670e6ee0f02a5840e2 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 21 Apr 2018 19:31:46 +0200
Subject: [PATCH 23/23] Fix #153: changelog

---
 changes/changelog.d/153.feature | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changes/changelog.d/153.feature

diff --git a/changes/changelog.d/153.feature b/changes/changelog.d/153.feature
new file mode 100644
index 000000000..1a2e1b92e
--- /dev/null
+++ b/changes/changelog.d/153.feature
@@ -0,0 +1 @@
+Can now import files in-place from the CLI importe (#155)