Fixing multiple caves with same kataser no

core/models_caves.py

@@ -5,6 +5,7 @@ import logging
 import re
 import json
 from subprocess import call
+from collections import defaultdict
 
 from urllib.parse import urljoin
 
@@ -21,6 +22,7 @@ from django.template import Context, loader
 
 from troggle.core.models import TroggleModel, Person, Expedition
 from troggle.core.models_survex import SurvexStation
+from troggle.core.models import DataIssue
 
 class Area(TroggleModel):
     short_name = models.CharField(max_length=100)
@@ -35,7 +37,7 @@ class Area(TroggleModel):
             return str(self.short_name)
 
     def kat_area(self):
-        if self.short_name in ["1623", "1626"]:
+        if self.short_name in ["1623", "1626", "1624", "1627"]:
             return self.short_name
         elif self.parent:
             return self.parent.kat_area()
@@ -515,30 +517,47 @@ def get_scan_path(instance, filename):
     return os.path.join('./',year,year+r'#'+number,str(instance.contents)+str(instance.number_in_wallet)+r'.jpg')
 
 Gcavelookup = None
+Gcave_count = None
 def GetCaveLookup():
     """lookup function modelled on GetPersonExpeditionNameLookup
     repeated assignment each call, needs refactoring
+    
+    Does NOT detect duplicates! Needs fixing.
+    Needs to be a proper funciton that raises an exception if there is a duplicate.
+    OR we could set it to return None if there are duplictes, and require the caller to 
+    fall back on doing the actual database query it wants rathe rthna using  this cache shortcut
     """
     global Gcavelookup
     if Gcavelookup:
         return Gcavelookup
     Gcavelookup = {"NONEPLACEHOLDER": None}
+    global Gcave_count
+    Gcave_count  = defaultdict(int) # sets default value to int(0)
+    
     for cave in Cave.objects.all():
-        Gcavelookup[cave.official_name.lower()] = cave
+        key = cave.official_name.lower()
+        Gcavelookup[key] = cave
+        Gcave_count[key] += 1
         if cave.kataster_number:
-            Gcavelookup[cave.kataster_number] = cave
+            Gcavelookup[cave.kataster_number] = cave # DUPLICATE as we have 1623-55 and 1626-55
+            Gcave_count[cave.kataster_number]  += 1
         if cave.unofficial_number:
             Gcavelookup[cave.unofficial_number.lower()] = cave
+            Gcave_count[cave.unofficial_number.lower()] += 1
         if cave.filename:
             # this is the slug - usually..
             Gcavelookup[cave.filename.replace(".html","").lower()] = cave
+            Gcave_count[cave.filename.replace(".html","").lower()] += 1
         if cave.slug():
             slug = cave.slug()
             Gcavelookup[slug.lower()] = cave
+            Gcave_count[slug.lower()] += 1
     # These are exact matches! edit to check for prefix only!
     # mostly taken from expoweb/noinfo/cave-number-index 
     # and Becka's email of 25 may 2020 on new kataster numbers
     # this should be re-done as a JSON file upload
+    
+    # These might alse create more duplicate entries, so re-write it to check
     Gcavelookup["1987-02"] = Gcavelookup["267"]
     Gcavelookup["1990-01"] = Gcavelookup["171"]
     Gcavelookup["1990-02"] = Gcavelookup["172"]
@@ -624,7 +643,7 @@ def GetCaveLookup():
     Gcavelookup["2016-jb-01"] = Gcavelookup["289"]
     Gcavelookup["2017-pw-01"] = Gcavelookup["277"]
     Gcavelookup["2018-dm-07"] = Gcavelookup["359"]
-    Gcavelookup["2017_cucc_24"] = Gcavelookup["291"]
+    Gcavelookup["2017_cucc_24"] = Gcavelookup["291"] # note _ not - here
     Gcavelookup["2017_cucc_23"] = Gcavelookup["295"]
     Gcavelookup["2017_cucc_28"] = Gcavelookup["290"]
     Gcavelookup["bs17"] = Gcavelookup["283"]
@@ -680,4 +699,11 @@ def GetCaveLookup():
     with open("cave-lookup.json", 'w') as f:
         json.dump(addmore, f)
         
+    for c in Gcave_count:
+        if Gcave_count[c] > 1:
+            message = " ** Duplicate cave id: {}:{}:{}".format(Gcave_count[c], Gcavelookup[c], c)
+            #print(message)
+            #DataIssue.objects.create(parser='caves', message=message)
+            # logdataissues[Gcavelookup[c]]=message # pending troggle-wide issues logging system
+
     return Gcavelookup

core/view_surveys.py

@@ -15,17 +15,20 @@ import parsers.surveys
 '''Some of these views serve files as binary blobs, and simply set the mime type based on the file extension,
 as does the urls.py dispatcher which sends them here. Here they should actually have the filetype checked 
 by looking inside the file before being served.
+
+ need to check if inavlid query string is invalid, or produces multiple replies
+ and render a user-friendly error page.
 '''
 
 def surveyscansfolder(request, path):
     #print [ s.walletname  for s in ScansFolder.objects.all() ]
-    scansfolder = ScansFolder.objects.get(walletname=urlunquote(path))
+    scansfolder = ScansFolder.objects.get(walletname=urlunquote(path))  # need to check if inavlid query string and produce friendly error
     return render(request, 'scansfolder.html', { 'scansfolder':scansfolder, 'settings': settings })
 
 def surveyscansingle(request, path, file):
     '''sends a single binary file to the user,
     '''
-    scansfolder = ScansFolder.objects.get(walletname=urlunquote(path))
+    scansfolder = ScansFolder.objects.get(walletname=urlunquote(path)) # need to check if inavlid query string and produce friendly error
     singlescan = SingleScan.objects.get(scansfolder=scansfolder, name=file)
     # print(" - surveyscansingle {}:{}:{}:".format(path, file, flatviews.getmimetype(file)))
     return HttpResponse(content=open(singlescan.ffile,"rb"), content_type=flatviews.getmimetype(file)) # any type of image
@@ -44,12 +47,12 @@ def tunneldata(request):
 def tunnelfilesingle(request, path):
     '''sends a single binary file to the user, We should have a renderer that syntax-colours this Tunnel xml 
     '''
-    tunnelfile = TunnelFile.objects.get(tunnelpath=urlunquote(path))
+    tunnelfile = TunnelFile.objects.get(tunnelpath=urlunquote(path)) # need to check if inavlid query string and produce friendly error
     tfile = Path(settings.TUNNEL_DATA, tunnelfile.tunnelpath)
     return HttpResponse(content=open(tfile), content_type="text/xhtml") # for display not download
     
 def tunnelfileupload(request, path):
-    tunnelfile = TunnelFile.objects.get(tunnelpath=urlunquote(path))
+    tunnelfile = TunnelFile.objects.get(tunnelpath=urlunquote(path)) # need to check if inavlid query string and produce friendly error
     tfile = Path(settings.TUNNEL_DATA, tunnelfile.tunnelpath)
     
     project, user, password, tunnelversion = request.POST["tunnelproject"], request.POST["tunneluser"], request.POST["tunnelpassword"], request.POST["tunnelversion"]

core/views_caves.py

@@ -11,11 +11,12 @@ from django.conf import settings
 from django.urls import reverse
 from django.http import HttpResponse, HttpResponseRedirect
 from django.shortcuts import get_object_or_404, render
+from django.core.exceptions import ObjectDoesNotExist, MultipleObjectsReturned
 
 import troggle.settings as settings
-import troggle.core.models as models
+#import troggle.core.models as models
-from troggle.core.models import Expedition
+from troggle.core.models import Expedition, DataIssue
-from troggle.core.models_caves import CaveSlug, Cave, CaveAndEntrance, QM, EntranceSlug, Entrance, Area, SurvexStation
+from troggle.core.models_caves import CaveSlug, Cave, CaveAndEntrance, QM, EntranceSlug, Entrance, Area, SurvexStation, GetCaveLookup
 from troggle.core.forms import CaveForm, CaveAndEntranceFormSet, VersionControlCommentForm, EntranceForm, EntranceLetterForm
 from troggle.helper import login_required_if_public
 
@@ -41,14 +42,14 @@ class MapLocations(object):
                     k = ent.caveandentrance_set.all()[0].cave
                 except:
                     message = " ! Failed to get Cave linked to Entrance:{} from:{} best:{}".format(ent.name, ent.filename, ent.best_station())
-                    models.DataIssue.objects.create(parser='entrances', message=message)
+                    DataIssue.objects.create(parser='entrances', message=message)
                     print(message)
                     raise
                 try:
                     areaName = k.getArea().short_name
                 except:
                     message = " ! Failed to get Area on cave '{}' linked to Entrance:{} from:{} best:{}".format(cave, ent.name, ent.filename, ent.best_station())
-                    models.DataIssue.objects.create(parser='entrances', message=message)
+                    DataIssue.objects.create(parser='entrances', message=message)
                     print(message)
                     raise
                 self.p.append((ent.best_station(), "%s-%s" % (areaName, str(ent)[5:]), ent.needs_surface_work(), str(ent)))
@@ -58,13 +59,23 @@ class MapLocations(object):
         return "{} map locations".format(len(self.p))
 
 def getCave(cave_id):
-    """Returns a cave object when given a cave name or number. It is used by views including cavehref, ent, and qm."""
+    '''Returns a cave object when given a cave name or number. It is used by views including cavehref, ent, and qm.
-    # REPLACE OR MERGE this with other functions that do exactly th same thing!
+    
+    TO DO: search GCavelookup first, which should raise a MultpleObjectsReturned exception if there
+    are duplicates'''
     try:
         cave = Cave.objects.get(kataster_number=cave_id)
+    except Cave.MultipleObjectsReturned as ex:
+        raise MultipleObjectsReturned("Duplicate kataster number")  from ex # propagate this up
+
     except Cave.DoesNotExist:
-        cave = Cave.objects.get(unofficial_number=cave_id)
+        Gcavelookup = GetCaveLookup() # dictionary makes strings to Cave objects
-    return cave
+        if cave_id in Gcavelookup:
+            return Gcavelookup[cave_id] 
+        else: 
+             raise ObjectDoesNotExist("No cave found with this identifier in any id field")
+    except:
+        raise ObjectDoesNotExist("No cave found with this identifier in any id field")
 
 def pad5(x):
    return "0" * (5 -len(x.group(0))) + x.group(0) 
@@ -75,7 +86,7 @@ def numericalcmp(x, y):
 
 def caveKey(x):
     """python3 function for sort. Done in a hurry.
-    Note that cave kataster numbers are not generally integers.
+    Note that cave kataster numbers are not always integers.
     This needs to be fixed make a decent sort order.
     """ 
     if not x.kataster_number:
@@ -107,7 +118,12 @@ def caveindex(request):
     return render(request,'caveindex.html', {'caves1623': caves1623, 'caves1626': caves1626, 'notablecaves':getnotablecaves(), 'cavepage': True})
 
 def cave3d(request, cave_id=''):
+    try:
         cave = getCave(cave_id)
+    except Cave.MultipleObjectsReturned: # entirely the wrong action, REPLACE with the right display
+        caves = Cave.objects.filter(kataster_number=cave_id) 
+        return render(request, 'svxcaveseveral.html', {'settings': settings, "caves":caves })
+    
     survexfilename = settings.SURVEX_DATA + cave.survex_file
     threedfilename = settings.THREEDCACHEDIR + '%s.3d' % cave_id
     if True or os.path.getmtime(survexfilename) > os.path.getmtime(threedfilename):
@@ -121,7 +137,14 @@ def cave3d(request, cave_id=''):
     return response
 
 def cave(request, cave_id='', offical_name=''):
+    try:
         cave=getCave(cave_id) 
+    except MultipleObjectsReturned:
+        caves = Cave.objects.filter(kataster_number=cave_id) 
+        return render(request, 'svxcaveseveral.html', {'settings': settings, "caves":caves }) # not the right template, needs a specific one
+    except ObjectDoesNotExist:
+        return render(request, 'svxcavesingle404.html', {'settings': settings, "cave":cave_id })
+
     if cave.non_public and settings.PUBLIC_SITE and not request.user.is_authenticated():
         return render(request, 'nonpublic.html', {'instance': cave, 'cavepage': True, 'cave_id': cave_id})
     else:
@@ -256,6 +279,9 @@ def qm(request,cave_id,qm_id,year,grade=None):
     try:
         qm=getCave(cave_id).get_QMs().get(number=qm_id,found_by__date__year=year) 
         return render(request,'qm.html',locals())
+    except Cave.MultipleObjectsReturned: # entirely the wrong action, REPLACE with the right display
+        caves = Cave.objects.filter(kataster_number=cave_id) 
+        return render(request, 'svxcaveseveral.html', {'settings': settings, "caves":caves })
 
     except QM.DoesNotExist:
         url=urllib.parse.urljoin(settings.URL_ROOT, r'/admin/core/qm/add/'+'?'+  r'number=' + qm_id)

core/views_survex.py

@@ -377,6 +377,8 @@ def survexcavesingle(request, survex_cave):
     '''parsing all the survex files of a single cave and showing that it's consistent and can find all 
     the files and people.   Should explicity fix the kataster number thing.
     kataster numbers are not unique across areas. Fix this.
+    
+    Should use getCave() from models_caves
     '''
     sc = survex_cave
     try:

flatpages/views.py

@@ -13,7 +13,6 @@ from django.views.decorators.csrf import ensure_csrf_cookie
 import django.forms as forms
 
 from troggle.helper import login_required_if_public
-#from troggle.flatpages.models import Redirect, EntranceRedirect
 from troggle.core.models_caves import Cave
 import troggle.core.views_caves
 import troggle.settings as settings
@@ -30,14 +29,14 @@ def expofilessingle(request, filepath):
     fn = Path(settings.EXPOFILES,filepath)
     if fn.is_dir():
         return expofilesdir(request, Path(fn), Path(filepath))
-    print(" - expofilessingle {}:{}:{}:".format(filepath, fn, getmimetype(fn)))
+    # print(" - expofilessingle {}:{}:{}:".format(filepath, fn, getmimetype(fn)))
     return HttpResponse(content=open(fn, "rb"),content_type=getmimetype(filepath)) # any file
 
 def expofilesdir(request, dirpath, filepath):
     '''does a directory display. If there is an index.html file we should display that.
     - dirpath is a Path() and it does not have /expofiles/ in it
     '''
-    print(" - expofilesdir {}".format(dirpath))
+    # print(" - expofilesdir {}".format(dirpath))
     urlpath = 'expofiles' / Path(filepath)
     fileitems = []
     diritems = []
@@ -53,31 +52,19 @@ def expofilesdir(request, dirpath, filepath):
 
 def flatpage(request, path):
     '''Either renders an HTML page from expoweb with all the menus,
-    or serves an unadorned binary file with mime type'''
+    or serves an unadorned binary file with mime type
-    print(" - FLATPAGES delivering the file: {} as MIME type: {}".format(path,getmimetype(path)),flush=True)
+    
+    This is a horrible mess and some code is redundant and unreachable because of urls.py setup
+    '''
+    # print(" - FLATPAGES delivering the file: {} as MIME type: {}".format(path,getmimetype(path)),flush=True)
 
     if path.startswith("noinfo") and settings.PUBLIC_SITE and not request.user.is_authenticated():
-        print((" - FLATPAGES redirect to logon: flat path noinfo", path))
+        # print((" - FLATPAGES redirect to logon: flat path noinfo", path))
         return HttpResponseRedirect(urljoin(reverse("auth_login"),'?next={}'.format(request.path)))
 
-    try:
-        r = Cave.objects.get(url = path)
-        return troggle.core.views_caves.caveSlug(request, r.slug())
-    except Cave.DoesNotExist:
-        pass
-    except:
-        #print(" ! FAILED to get only one cave per slug for: "+path)
-        # we should do this proper;ly, not this hack that returns the first cave that matches
-        caves = Cave.objects.all().filter(url = path)
-        for c in caves:
-            print(path, c.slug())
-            if c.slug() != None:
-                return troggle.core.views_caves.caveSlug(request, c.slug())
-        pass
-
     expowebpath = Path(settings.EXPOWEB)
     if path.endswith("/") or path == "":
-        print(" - FLATPAGES the file: {} ENDSWITH ...".format(path))
+        # print(" - FLATPAGES the file: {} ENDSWITH ...".format(path))
 
         try:
             o = open(os.path.normpath(expowebpath / path / "index.html"), "rb")
@@ -89,25 +76,25 @@ def flatpage(request, path):
             except IOError:
                 return render(request, 'pagenotfound.html', {'path': path})
     else:
-        print(" - FLATPAGES the file: '{}'  ...".format(path))
+        # print(" - FLATPAGES the file: '{}'  ...".format(path))
         if path.startswith('site_media'):
-            print(" - MEDIA_ROOT: {}  ...{}".format(settings.MEDIA_ROOT, path))
+            # print(" - MEDIA_ROOT: {}  ...{}".format(settings.MEDIA_ROOT, path))
             path = path.replace("site_media", settings.MEDIA_ROOT)
             filetobeopened = os.path.normpath(path)
         elif path.startswith("static"):
-            print(" - STATIC_ROOT: {}  ...{}".format(settings.MEDIA_ROOT, path))
+            # print(" - STATIC_ROOT: {}  ...{}".format(settings.MEDIA_ROOT, path))
             path = path.replace("static", settings.MEDIA_ROOT)
             filetobeopened = os.path.normpath(path)
         else:
-            print(" - NO    _ROOT: {}  ...".format(expowebpath))
+            # print(" - NO    _ROOT: {}  ...".format(expowebpath))
             filetobeopened = os.path.normpath(expowebpath / path)
         
-        print(" - FLATPAGES full path : {}  ...".format(filetobeopened))
+        # print(" - FLATPAGES full path : {}  ...".format(filetobeopened))
         try:
             o = open(filetobeopened, "rb")
-            print(" - FLATPAGES full path no error: {}  ...".format(filetobeopened))
+            #print(" - FLATPAGES full path no error: {}  ...".format(filetobeopened))
         except IOError:
-            print(" - FLATPAGES ERROR: {}  ...".format(filetobeopened))
+            #print(" - FLATPAGES ERROR: {}  ...".format(filetobeopened))
             #o.close() 
             return render(request, 'pagenotfound.html', {'path': path})
  
@@ -147,7 +134,7 @@ def flatpage(request, path):
         return render(request, 'flatpage.html', {'editable': editable, 'path': path, 'title': title, 
                     'body': body, 'homepage': (path == "index.htm"), 'has_menu': has_menu})
     else:
-        print(" - FLATPAGES delivering the file: {} as MIME type: {}".format(path,getmimetype(path)))
+        # print(" - FLATPAGES delivering the file: {} as MIME type: {}".format(path,getmimetype(path)))
         return HttpResponse(content=open(filetobeopened, "rb"), content_type=getmimetype(path))
         #return HttpResponse(content=open(singlescan.ffile,"rb"), content_type=getmimetype(path))