GNOME.org
 

[damned-lies] Don't store absolute paths in PoFile.path fields

commit 28062e832487aaf68392230383e5f1de196a6c7a
Author: Claude Paroz <claude 2xlibre net>
Date:   Thu Sep 12 10:44:24 2019 +0200

    Don't store absolute paths in PoFile.path fields

 stats/migrations/0017_pofile_path_relative.py    | 30 +++++++++++++++++++++
 stats/models.py                                  | 28 ++++++++++++-------
 vertimus/migrations/0005_action_proxy_pofile.py  | 34 ++++++++++++++++++++++++
 vertimus/migrations/0006_pofile_path_relative.py | 28 +++++++++++++++++++
 vertimus/models.py                               | 25 +++++++++++++----
 vertimus/views.py                                |  8 +++---
 6 files changed, 135 insertions(+), 18 deletions(-)
---
diff --git a/stats/migrations/0017_pofile_path_relative.py b/stats/migrations/0017_pofile_path_relative.py
new file mode 100644
index 00000000..48a468eb
--- /dev/null
+++ b/stats/migrations/0017_pofile_path_relative.py
@@ -0,0 +1,30 @@
+import os
+from django.conf import settings
+from django.db import migrations
+
+
+def strip_path_prefix(apps, schema_editor):
+    Statistics = apps.get_model("stats", "Statistics")
+    scratch_dir = os.path.basename(settings.SCRATCHDIR)
+
+    def strip_path(stat):
+        old_path = stat.full_po.path
+        stat.full_po.path = old_path.split('/' + scratch_dir + '/')[1]
+        stat.full_po.save()
+        if stat.part_po is not None and stat.part_po.path.startswith('/'):
+            old_path = stat.part_po.path
+            stat.part_po.path = old_path.split('/' + scratch_dir + '/')[1]
+            stat.part_po.save()
+        
+    
+    for stat in Statistics.objects.filter(full_po__isnull=False, full_po__path__startswith='/'):
+        strip_path(stat)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('stats', '0016_removed_bugs_fields'),
+    ]
+
+    operations = [migrations.RunPython(strip_path_prefix, migrations.RunPython.noop)]
diff --git a/stats/models.py b/stats/models.py
index 19d74d67..85e0eb14 100644
--- a/stats/models.py
+++ b/stats/models.py
@@ -1209,7 +1209,15 @@ class PoFile(models.Model):
 
     @property
     def url(self):
-        return utils.url_join(settings.MEDIA_URL, settings.UPLOAD_DIR, os.path.basename(self.path))
+        return utils.url_join(settings.MEDIA_URL, settings.UPLOAD_DIR, self.filename())
+
+    @property
+    def prefix(self):
+        return settings.SCRATCHDIR
+
+    @property
+    def full_path(self):
+        return os.path.join(self.prefix, self.path)
 
     def filename(self):
         return os.path.basename(self.path)
@@ -1496,7 +1504,7 @@ class Statistics(models.Model):
         Also try to generate a "reduced" po file.
         """
         if file_path is None and self.full_po:
-            file_path = self.full_po.path
+            file_path = self.full_po.full_path
         stats = utils.po_file_stats(file_path)
         for err in stats['errors']:
             self.set_error(*err)
@@ -1518,10 +1526,11 @@ class Statistics(models.Model):
             df = None
         fig_stats = utils.get_fig_stats(file_path, df)
 
+        relpath = os.path.relpath(file_path, settings.SCRATCHDIR)
         if not self.full_po:
-            self.full_po = PoFile.objects.create(path=file_path)
+            self.full_po = PoFile.objects.create(path=relpath)
             self.save()
-        self.full_po.path = str(file_path)
+        self.full_po.path = relpath
         self.full_po.translated = int(stats['translated'])
         self.full_po.fuzzy = int(stats['fuzzy'])
         self.full_po.untranslated = int(stats['untranslated'])
@@ -1546,10 +1555,10 @@ class Statistics(models.Model):
             if (self.full_po.fuzzy + self.full_po.untranslated) > 0 and not self.branch.is_archive_only() 
and self.domain.red_filter != '-':
                 # Generate partial_po and store partial stats
                 if self.full_po.path.endswith('.pot'):
-                    part_po_path = Path(self.full_po.path[:-3] + "reduced.pot")
+                    part_po_path = Path(self.full_po.full_path[:-3] + "reduced.pot")
                 else:
-                    part_po_path = Path(self.full_po.path[:-3] + ".reduced.po")
-                utils.po_grep(self.full_po.path, str(part_po_path), self.domain.red_filter)
+                    part_po_path = Path(self.full_po.full_path[:-3] + ".reduced.po")
+                utils.po_grep(self.full_po.full_path, str(part_po_path), self.domain.red_filter)
                 part_stats = utils.po_file_stats(part_po_path)
                 if (part_stats['translated'] + part_stats['fuzzy'] + part_stats['untranslated'] ==
                         stats['translated'] + stats['fuzzy'] + stats['untranslated']):
@@ -1558,10 +1567,11 @@ class Statistics(models.Model):
                     part_po_path.unlink()
                 else:
                     utils.add_custom_header(part_po_path, "X-DamnedLies-Scope", "partial")
+                    relpath = os.path.relpath(part_po_path, settings.SCRATCHDIR)
                     if not self.part_po or self.part_po == self.full_po:
-                        self.part_po = PoFile.objects.create(path=part_po_path)
+                        self.part_po = PoFile.objects.create(path=relpath)
                         self.save()
-                    self.part_po.path = str(part_po_path)
+                    self.part_po.path = relpath
                     self.part_po.translated = part_stats['translated']
                     self.part_po.fuzzy = part_stats['fuzzy']
                     self.part_po.untranslated = part_stats['untranslated']
diff --git a/vertimus/migrations/0005_action_proxy_pofile.py b/vertimus/migrations/0005_action_proxy_pofile.py
new file mode 100644
index 00000000..f1b32e93
--- /dev/null
+++ b/vertimus/migrations/0005_action_proxy_pofile.py
@@ -0,0 +1,34 @@
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('stats', '0001_initial'),
+        ('vertimus', '0004_tables_to_utf8mb4'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='MergedPoFile',
+            fields=[
+            ],
+            options={
+                'proxy': True,
+                'indexes': [],
+                'constraints': [],
+            },
+            bases=('stats.pofile',),
+        ),
+        migrations.AlterField(
+            model_name='action',
+            name='merged_file',
+            field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, 
to='vertimus.MergedPoFile'),
+        ),
+        migrations.AlterField(
+            model_name='actionarchived',
+            name='merged_file',
+            field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, 
to='vertimus.MergedPoFile'),
+        ),
+    ]
diff --git a/vertimus/migrations/0006_pofile_path_relative.py 
b/vertimus/migrations/0006_pofile_path_relative.py
new file mode 100644
index 00000000..a2603c14
--- /dev/null
+++ b/vertimus/migrations/0006_pofile_path_relative.py
@@ -0,0 +1,28 @@
+import os
+from django.conf import settings
+from django.db import migrations
+
+
+def strip_path_prefix(apps, schema_editor):
+    Action = apps.get_model("vertimus", "Action")
+    ActionArchived = apps.get_model("vertimus", "Action")
+    media_dir = os.path.basename(settings.MEDIA_ROOT)
+
+    def strip_path(action):
+        old_path = action.merged_file.path
+        action.merged_file.path = old_path.split('/' + media_dir + '/')[1]
+        action.merged_file.save()
+    
+    for act in Action.objects.filter(merged_file__isnull=False):
+        strip_path(act)
+    for act in ActionArchived.objects.filter(merged_file__isnull=False):
+        strip_path(act)
+ 
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('vertimus', '0005_action_proxy_pofile'),
+    ]
+
+    operations = [migrations.RunPython(strip_path_prefix, migrations.RunPython.noop)]
diff --git a/vertimus/models.py b/vertimus/models.py
index f97e6920..ff140a97 100644
--- a/vertimus/models.py
+++ b/vertimus/models.py
@@ -334,6 +334,15 @@ def generate_upload_filename(instance, filename):
     return "%s/%s" % (settings.UPLOAD_DIR, new_filename)
 
 
+class MergedPoFile(PoFile):
+    class Meta:
+        proxy = True
+
+    @property
+    def prefix(self):
+        return settings.MEDIA_ROOT
+
+
 class ActionAbstract(models.Model):
     """ Common model for Action and ActionArchived """
     state_db = models.ForeignKey(State, on_delete=models.CASCADE)
@@ -344,7 +353,7 @@ class ActionAbstract(models.Model):
     comment = models.TextField(blank=True, null=True)
     sent_to_ml = models.BooleanField(default=False)
     file = models.FileField(upload_to=generate_upload_filename, blank=True, null=True)
-    merged_file = models.OneToOneField(PoFile, blank=True, null=True, on_delete=models.SET_NULL)
+    merged_file = models.OneToOneField(MergedPoFile, blank=True, null=True, on_delete=models.SET_NULL)
 
     # A comment or a file is required
     arg_is_required = False
@@ -369,6 +378,10 @@ class ActionAbstract(models.Model):
     def most_uptodate_file(self):
         return self.merged_file if self.merged_file else self.file
 
+    @property
+    def most_uptodate_filepath(self):
+        return self.merged_file.full_path if self.merged_file else self.file.path
+
     @property
     def can_build(self):
         return (
@@ -505,10 +518,12 @@ class Action(ActionAbstract):
             return
         if not self.merged_file:
             merged_path = "%s.merged.po" % self.file.path[:-3]
-            self.merged_file = PoFile.objects.create(path=merged_path)
+            self.merged_file = MergedPoFile.objects.create(
+                path=os.path.relpath(merged_path, settings.MEDIA_ROOT)
+            )
             self.save()
             return # post_save will call merge_file_with_pot again
-        merged_path = self.merged_file.path
+        merged_path = self.merged_file.full_path
         command = ['msgmerge', '--previous', '-o', merged_path, self.file.path, str(pot_file)]
         run_shell_command(command)
         # If uploaded file is reduced, run po_grep *after* merge
@@ -816,8 +831,8 @@ def delete_action_files(sender, instance, **kwargs):
         return
     if instance.file.path.endswith('.po'):
         if instance.merged_file:
-            if os.access(instance.merged_file.path, os.W_OK):
-                 os.remove(instance.merged_file.path)
+            if os.access(instance.merged_file.full_path, os.W_OK):
+                 os.remove(instance.merged_file.full_path)
     if os.access(instance.file.path, os.W_OK):
          os.remove(instance.file.path)
     html_dir = Path(settings.SCRATCHDIR, 'HTML', str(instance.pk))
diff --git a/vertimus/views.py b/vertimus/views.py
index 1d43fc5a..7ff78bf9 100644
--- a/vertimus/views.py
+++ b/vertimus/views.py
@@ -170,7 +170,7 @@ def vertimus_diff(request, action_id_1, action_id_2, level):
     action_1 = get_object_or_404(ActionReal, pk=action_id_1)
     state = action_1.state_db
 
-    file_path_1 = action_1.most_uptodate_file.path
+    file_path_1 = action_1.most_uptodate_filepath
     reduced = is_po_reduced(file_path_1)
     if not os.path.exists(file_path_1):
         raise Http404("File not found")
@@ -184,7 +184,7 @@ def vertimus_diff(request, action_id_1, action_id_2, level):
     if action_id_2 not in (None, 0):
         # 1) id_2 specified in URL
         action_2 = get_object_or_404(ActionReal, pk=action_id_2)
-        file_path_2 = action_2.most_uptodate_file.path
+        file_path_2 = action_2.most_uptodate_filepath
         descr_2 = _('<a href="%(url)s">Uploaded file</a> by %(name)s on %(date)s') % {
             'url': action_2.most_uptodate_file.url,
             'name': action_2.person.name,
@@ -197,7 +197,7 @@ def vertimus_diff(request, action_id_1, action_id_2, level):
             action_2 = action_1.get_previous_action_with_po()
 
         if action_2:
-            file_path_2 = action_2.most_uptodate_file.path
+            file_path_2 = action_2.most_uptodate_filepath
             descr_2 = _('<a href="%(url)s">Uploaded file</a> by %(name)s on %(date)s') % {
                 'url': action_2.most_uptodate_file.url,
                 'name': action_2.person.name,
@@ -271,7 +271,7 @@ class PoFileActionBase(View):
         if self.kwargs.get('action_pk'):
             self.action = get_object_or_404(Action, pk=self.kwargs['action_pk'])
             if self.action.has_po_file():
-                pofile = self.action.most_uptodate_file.path
+                pofile = self.action.most_uptodate_filepath
         elif self.kwargs.get('stats_pk'):
             stats = get_object_or_404(Statistics, pk=self.kwargs['stats_pk'])
             pofile = stats.po_path()
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]