From 47039ec35c0f0d4c2cd385bb4e2321714c1bc28a Mon Sep 17 00:00:00 2001 From: Damien De Paoli Date: Fri, 28 Jan 2022 15:52:35 +1100 Subject: [PATCH] partial fix for various BUGs with going past the end of a viewing list... we now keep num_entries (of files) in pa_user_state, and use it to stop going before first or past last entry, even from viewlist, so this fixes many isseus... Final bug(s) are relating to multiple Dirs in a Path and its feeling too complex for no real gain, going to remove the feature, but for now, this version works / can be made to PROD --- BUGs | 3 +- TODO | 9 +++-- files.py | 110 ++++++++++++++++++++++++++++++++++++++--------------- states.py | 69 ++++++++++++++++++++------------- tables.sql | 4 +- 5 files changed, 133 insertions(+), 62 deletions(-) diff --git a/BUGs b/BUGs index 3dc2416..947e58e 100644 --- a/BUGs +++ b/BUGs @@ -1,3 +1,4 @@ ### Next: 83 BUG-60: entries per page in flat view will get how_many from each top-level dir in PATH (not a big issue, but it is a little misleading) -BUG-82: occasionally, the face_locn data in the DB is {} not [], not sure what sequence causes this +BUG-82: occasionally, the face_locn data in the DB is {} not [], and if faces in data viewer crashes -- 'next' too fast and offset gets larger than the end of the eids list + - can't next to end for flat view of PATH with multiple Dirs (but-60) above... (list of ents, if > how_many, just should not include next dir content) diff --git a/TODO b/TODO index 14b28cf..8d7b362 100644 --- a/TODO +++ b/TODO @@ -1,7 +1,6 @@ ## GENERAL - *** could get better AI optim, by keeping track of just new files since scan (even if we did this from the DB), - then we could jsut feed those eid's explicitly into a 'run_ai_on_new_files' :) -- maybe particularly - if count('new files') < say 1000 do eids, otherwise do path AND no new refimgs + * DECIDED TO DITCH multiple Dirs per Path, just adds complexity and not needed + - will address BUG-60, and last bit of BUG-83 * browser back/forward buttons dont work -- use POST -> redirect to GET * viewlist @@ -27,6 +26,10 @@ * refimg locns can lose an array idx of 0 always. + * could get better AI optim, by keeping track of just new files since scan (even if we did this from the DB), + then we could just feed those eid's explicitly into a 'run_ai_on_new_files' :) -- maybe particularly + if count('new files') < say 1000 do eids, otherwise do path AND no new refimgs + * search allow noo * delete folder diff --git a/files.py b/files.py index 57427a1..076a5cf 100644 --- a/files.py +++ b/files.py @@ -185,6 +185,8 @@ def UpdatePref( pref, OPT ): pref.first_eid=OPT.first_eid if OPT.last_eid>0: pref.last_eid=OPT.last_eid + if OPT.num_entries>0: + pref.num_entries=OPT.num_entries db.session.add(pref) db.session.commit() @@ -193,6 +195,7 @@ def UpdatePref( pref, OPT ): ################################################################################ def GetEntriesInFlatView( OPT, prefix ): entries=[] + num_entries=0 if OPT.noo == "Oldest": entries+=Entry.query.join(File).join(EntryDirLink).join(Dir).join(PathDirLink).join(Path).filter(Path.path_prefix==prefix).order_by(File.year,File.month,File.day,Entry.name).offset(OPT.offset).limit(OPT.how_many).all() @@ -208,17 +211,21 @@ def GetEntriesInFlatView( OPT, prefix ): last_entry=Entry.query.join(File).join(EntryDirLink).join(Dir).join(PathDirLink).join(Path).filter(Path.path_prefix==prefix).order_by(Entry.name.desc()).limit(1) if OPT.first_eid == 0 and OPT.offset == 0 and len(entries): OPT.first_eid = entries[0].id - - # FIXME: would prefer to only get this if needed (e.g. when last_eid is 0), - # but with multiple dirs in say import path, then we only set last_eid on - # the first dir... need to to think this through better, pass param maybe? - le=last_entry.all() - if len(le): - OPT.last_eid = le[0].id - pref=PA_UserState.query.filter(PA_UserState.pa_user_dn==current_user.dn,PA_UserState.path_type==OPT.path_type).first() - UpdatePref( pref, OPT ) - return entries + num_entries=Entry.query.join(File).join(EntryDirLink).join(Dir).join(PathDirLink).join(Path).filter(Path.path_prefix==prefix).count() + + print( f"GetEntriesInFlatView - pfx={prefix}, ne={num_entries}" ) + + for e in entries: + print( f"e={e.id}- {e.name}" ) + + if OPT.last_eid==0: + le=last_entry.all() + if len(le): + OPT.last_eid = le[0].id + print("1st run, fe={OPT.first_eid}, le={OPT.last_eid}" ) + + return entries, num_entries ################################################################################ # GetEntriesInFolderView: func. to retrieve DB entries appropriate for folder view @@ -226,12 +233,13 @@ def GetEntriesInFlatView( OPT, prefix ): ################################################################################ def GetEntriesInFolderView( OPT, prefix ): entries=[] + num_entries=0 # okay the root cwd is fake, so treat it specially - its Dir can be found by path with dir.rel_path='' if os.path.dirname(OPT.cwd) == 'static': dir=Entry.query.join(Dir).join(PathDirLink).join(Path).filter(Dir.rel_path=='').filter(Path.path_prefix==prefix).order_by(Entry.name).first() # this can occur if the path in settings does not exist as it wont be in # the DB if not dir: - return entries + return entries, num_entries # although this is 1 entry, needs to come back via all() to be iterable entries+= Entry.query.filter(Entry.id==dir.id).all() else: @@ -242,7 +250,7 @@ def GetEntriesInFolderView( OPT, prefix ): dir=Entry.query.join(Dir).join(PathDirLink).join(Path).filter(Dir.rel_path==rp).filter(Path.path_prefix==prefix).order_by(Entry.name).first() # this can occur if the path in settings does not exist as it wont be in # the DB if not dir: - return entries + return entries, 0 if OPT.noo == "Z to A" or OPT.noo == "Newest": entries+= Entry.query.join(EntryDirLink).join(FileType).filter(EntryDirLink.dir_eid==dir.id).filter(FileType.name=='Directory').order_by(Entry.name.desc()).all() # just do A to Z / Oldest by default or if no valid option @@ -251,28 +259,32 @@ def GetEntriesInFolderView( OPT, prefix ): # add any files at the current CWD (based on dir_eid in DB) if OPT.noo == "Oldest": - entries+=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(File.year,File.month,File.day,Entry.name).offset(OPT.offset).limit(OPT.how_many).all() - last_entry=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(File.year.desc(),File.month.desc(),File.day.desc(),Entry.name).limit(1) + file_entries=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(File.year,File.month,File.day,Entry.name).offset(OPT.offset).limit(OPT.how_many).all() + last_entry=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(File.year.desc(),File.month.desc(),File.day.desc(),Entry.name) elif OPT.noo == "Newest": - entries+=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(File.year.desc(),File.month.desc(),File.day.desc(),Entry.name).offset(OPT.offset).limit(OPT.how_many).all() - last_entry=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(File.year,File.month,File.day,Entry.name).limit(1) + file_entries=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(File.year.desc(),File.month.desc(),File.day.desc(),Entry.name).offset(OPT.offset).limit(OPT.how_many).all() + last_entry=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(File.year,File.month,File.day,Entry.name) elif OPT.noo == "Z to A": - entries+=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(Entry.name.desc()).offset(OPT.offset).limit(OPT.how_many).all() - last_entry=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(Entry.name).limit(1) + file_entries=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(Entry.name.desc()).offset(OPT.offset).limit(OPT.how_many).all() + last_entry=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(Entry.name) # just do A to Z by default or if no valid option else: - entries+=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(Entry.name).offset(OPT.offset).limit(OPT.how_many).all() - last_entry=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(Entry.name.desc()).limit(1) - if OPT.first_eid == 0 and OPT.offset == 0 and len(entries): - OPT.first_eid = entries[0].id - if OPT.last_eid == 0: - le=last_entry.all() + file_entries=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(Entry.name).offset(OPT.offset).limit(OPT.how_many).all() + last_entry=Entry.query.join(File).join(EntryDirLink).filter(EntryDirLink.dir_eid==dir.id).order_by(Entry.name.desc()) + if OPT.offset == 0 and len(file_entries): + OPT.first_eid = file_entries[0].id + if len(file_entries): + num_entries=last_entry.count() + le=last_entry.limit(1).all() if len(le): OPT.last_eid = le[0].id - pref=PA_UserState.query.filter(PA_UserState.pa_user_dn==current_user.dn,PA_UserState.path_type==OPT.path_type).first() - UpdatePref( pref, OPT ) - return entries + entries += file_entries; + print( f"FOLD, fe={OPT.first_eid}, le={OPT.last_eid}, ne={num_entries}" ) + for e in entries: + print( f"FOLD e={e.id} - {e.type.name} - {e.name}" ) + + return entries, num_entries ################################################################################ # /GetEntries -> helper function that Gets Entries for required files to show @@ -286,6 +298,7 @@ def GetEntries( OPT ): search_term = search_term.replace('AI:','') all_entries = Entry.query.join(File).distinct().join(FaceFileLink).join(Face).join(FaceRefimgLink).join(Refimg).join(PersonRefimgLink).join(Person).filter(Person.tag.ilike(f"%{search_term}%")).order_by(File.year.desc(),File.month.desc(),File.day.desc(),Entry.name).offset(OPT.offset).limit(OPT.how_many).all() if OPT.last_eid == 0: + OPT.num_entries = Entry.query.join(File).distinct().join(FaceFileLink).join(Face).join(FaceRefimgLink).join(Refimg).join(PersonRefimgLink).join(Person).filter(Person.tag.ilike(f"%{search_term}%")).count() last_entry=Entry.query.join(File).distinct().join(FaceFileLink).join(Face).join(FaceRefimgLink).join(Refimg).join(PersonRefimgLink).join(Person).filter(Person.tag.ilike(f"%{search_term}%")).order_by(File.year,File.month,File.day,Entry.name.desc()).limit(1).all() if len(last_entry): OPT.last_eid = last_entry[0].id @@ -327,6 +340,12 @@ def GetEntries( OPT ): order_desc=f"f.year desc, f.month desc, f.day desc, e.name" order_asc=f"f.year, f.month, f.day, e.name desc" + #num_entries + num_e_sql = f"select count(1) from ( {by_fname} union {by_dirname} union {by_ai} ) as foo" + num_e_result = db.engine.execute( num_e_sql ) + for res in num_e_result: + OPT.num_entries=res.count + last_entry_sql= f"{sel_no_order} order by {order_asc} limit 1" last_entry=db.engine.execute( last_entry_sql ) # can only be 1 due to limit above @@ -352,14 +371,38 @@ def GetEntries( OPT ): elif OPT.path_type == 'Bin': paths.append(SettingsRBPath()) + num_paths = len(paths) + + num_entries=0 + path_cnt=1 + + # if we have not set last_eid yet, then we need to 'reset' it during the + # path loop below (if we have more than one dir in (say) Import path) + if OPT.last_eid == 0 or OPT.folders: + update_last_eid = True + else: + update_last_eid = False for path in paths: if not os.path.exists(path): continue prefix = SymlinkName(OPT.path_type,path,path+'/') if OPT.folders: - entries+=GetEntriesInFolderView( OPT, prefix ) + tmp_ents, tmp_num_ents = GetEntriesInFolderView( OPT, prefix ) else: - entries+=GetEntriesInFlatView( OPT, prefix ) + tmp_ents, tmp_num_ents = GetEntriesInFlatView( OPT, prefix ) + entries += tmp_ents + num_entries += tmp_num_ents + # if we have another path, keep adding num_etnries, and last_eid is the last path, not this one, so reset to 0 + if update_last_eid and path_cnt < num_paths: + OPT.last_eid=0 + path_cnt += 1 + + if update_last_eid: + print( f"num_ent={OPT.num_entries} -- need to SAVE this in pa_user_state") + # find pref... via path_type if we are here + OPT.num_entries=num_entries + pref=PA_UserState.query.filter(PA_UserState.pa_user_dn==current_user.dn,PA_UserState.path_type==OPT.path_type).first() + UpdatePref( pref, OPT ) return entries ################################################################################ @@ -684,7 +727,14 @@ def view(id): for face in e.file_details.faces: face.locn = json.loads(face.locn) eids=eids.rstrip(",") - return render_template("viewer.html", current=int(id), eids=eids, objs=objs, OPT=OPT ) + # jic, sometimes we trip this, and rather than show broken pages / destroy + # face locn data, just warn & redirect + if id not in eids: + print( f"OPT={OPT}, id={id}, eids={eids}") + st.SetMessage("Sorry, viewing data is confused, cannot view this image now", "warning" ) + return redirect("/") + else: + return render_template("viewer.html", current=int(id), eids=eids, objs=objs, OPT=OPT ) ################################################################################## # /view/id -> grabs data from DB and views it (POST -> set state, redirect to GET) diff --git a/states.py b/states.py index 71832d1..6a100ff 100644 --- a/states.py +++ b/states.py @@ -34,9 +34,10 @@ class PA_UserState(db.Model): current = db.Column(db.Integer) first_eid = db.Column(db.Integer) last_eid = db.Column(db.Integer) + num_entries = db.Column(db.Integer) def __repr__(self): - return f"" + return f"" ################################################################################ @@ -53,7 +54,9 @@ class States(PA): self.current=0 self.first_eid=0 self.last_eid=0 + self.num_entries=0 + print( f"STATES() called, rp={request.path}, ref={request.referrer}" ) # this is any next/prev or noo, grouping, etc. change (so use referrer to work out what to do with this) # because this can happen on a view, or files_up, etc. change this FIRST if 'ChangeFileOpts' in request.path: @@ -141,6 +144,7 @@ class States(PA): self.current = pref.current self.first_eid = pref.first_eid self.last_eid = pref.last_eid + self.num_entries = pref.num_entries else: # retreive defaults from 'PAUser' where defaults are stored u=PAUser.query.filter(PAUser.dn==current_user.dn).one() @@ -149,24 +153,29 @@ class States(PA): self.offset=0 self.size=u.default_size if self.path_type == "View": + print("WE ARE A VIEW" ) self.root='static/' + self.orig_ptype - tmp=self.orig_ptype self.first_eid=orig_pref.first_eid self.last_eid=orig_pref.last_eid + self.num_entries=orig_pref.num_entries + self.noo=orig_pref.noo + self.folders=orig_pref.folders + print(f"orig_pref={orig_pref}" ) + print(f"self.folders={self.folders}" ) else: + print("not a view?" ) self.root='static/' + self.path_type - tmp=self.path_type - if tmp == 'Import': - self.noo = u.default_import_noo - self.folders = u.default_import_folders - elif tmp == 'Storage': - self.noo = u.default_storage_noo - self.folders = u.default_storage_folders - else: - # is a search so... - print( "For now, search defaults for noo / folders are hardcoded" ) - self.folders=False - self.noo = 'Oldest' + if self.path_type == 'Import': + self.noo = u.default_import_noo + self.folders = u.default_import_folders + elif self.path_type == 'Storage': + self.noo = u.default_storage_noo + self.folders = u.default_storage_folders + else: + # is a search so... + print( "For now, search defaults for noo / folders are hardcoded" ) + self.noo = 'Oldest' + self.folders=False self.cwd=self.root if not hasattr(self, 'orig_ptype'): @@ -177,7 +186,7 @@ class States(PA): # the above are defaults, if we are here, then we have current values, use them instead if they are set -- AI: searches dont set them so then we use those in the DB first if request.method=="POST": - if 'noo' in request.form: + if self.path_type != "View" and 'noo' in request.form: self.noo=request.form['noo'] if 'how_many' in request.form: self.how_many=request.form['how_many'] @@ -189,14 +198,15 @@ class States(PA): if 'size' in request.form: self.size = request.form['size'] # seems html cant do boolean, but uses strings so convert - if 'folders' not in request.form or request.form['folders'] == "False": - self.folders=False - elif request.form['folders'] == "True": - self.folders=True - # have to force grouping to None if we flick to folders from a flat - # view with grouping (otherwise we print out group headings for - # child content that is not in the CWD) - self.grouping=None + if self.path_type != "View" and 'folders' in request.form: + if request.form['folders'] == "False": + self.folders=False + else: + self.folders=True + # have to force grouping to None if we flick to folders from a flat + # view with grouping (otherwise we print out group headings for + # child content that is not in the CWD) + self.grouping=None if 'orig_url' in request.form: self.orig_url = request.form['orig_url'] @@ -209,10 +219,15 @@ class States(PA): self.fullscreen=False if 'prev' in request.form: self.offset -= int(self.how_many) + # just in case we hit prev too fast, stop this... if self.offset < 0: self.offset=0 if 'next' in request.form: - self.offset += int(self.how_many) + if (self.offset + int(self.how_many)) < self.num_entries: + self.offset += int(self.how_many) + else: + print( f"WARNING: next image requested, but would go past end of list? - ignore this" ) + print( f"DDP - offset={self.offset} + how_many={self.how_many} > num_entries={self.num_entries}" ) if 'current' in request.form: self.current = int(request.form['current']) @@ -222,7 +237,7 @@ class States(PA): pref=PA_UserState( pa_user_dn=current_user.dn, path_type=self.path_type, view_eid=self.view_eid, noo=self.noo, grouping=self.grouping, how_many=self.how_many, st_offset=self.offset, size=self.size, folders=self.folders, root=self.root, cwd=self.cwd, orig_ptype=self.orig_ptype, orig_search_term=self.orig_search_term, - orig_url=self.orig_url, current=self.current, first_eid=self.first_eid, last_eid=self.last_eid ) + orig_url=self.orig_url, current=self.current, first_eid=self.first_eid, last_eid=self.last_eid, num_entries=self.num_entries ) else: # update this pref with the values calculated above (most likely from POST to form) pref.pa_user_dn=current_user.dn @@ -239,8 +254,10 @@ class States(PA): pref.orig_ptype = self.orig_ptype pref.orig_search_term = self.orig_search_term pref.orig_url = self.orig_url + + # only passed in (at the moment) in viewlist pref.current = self.current - # first_eid and last_eid wont change in this func, set only in GetEntries() + # first_eid, last_eid, num_entries wont change in this func, set only in GetEntries() db.session.add(pref) db.session.commit() diff --git a/tables.sql b/tables.sql index eb87066..c240976 100644 --- a/tables.sql +++ b/tables.sql @@ -33,7 +33,7 @@ create table PA_USER( create table PA_USER_STATE ( ID integer, PA_USER_DN varchar(128), PATH_TYPE varchar(16), NOO varchar(16), GROUPING varchar(16), HOW_MANY integer, ST_OFFSET integer, SIZE integer, FOLDERS Boolean, FULLSCREEN Boolean, ROOT varchar, CWD varchar, VIEW_EID integer, ORIG_PTYPE varchar, ORIG_SEARCH_TERM varchar, - ORIG_URL varchar, CURRENT integer, FIRST_EID integer, LAST_EID integer, + ORIG_URL varchar, CURRENT integer, FIRST_EID integer, LAST_EID integer, NUM_ENTRIES integer, constraint FK_PA_USER_DN foreign key (PA_USER_DN) references PA_USER(DN), constraint PK_PA_USER_STATES_ID primary key(ID ) ); @@ -145,7 +145,7 @@ insert into PERSON values ( (select nextval('PERSON_ID_SEQ')), 'mum', 'Mandy', ' insert into PERSON values ( (select nextval('PERSON_ID_SEQ')), 'cam', 'Cameron', 'De Paoli' ); insert into PERSON values ( (select nextval('PERSON_ID_SEQ')), 'mich', 'Michelle', 'De Paoli' ); -- DEV(ddp): -insert into SETTINGS ( id, base_path, import_path, storage_path, recycle_bin_path, default_refimg_model, default_scan_model, default_threshold, scheduled_import_scan, scheduled_storage_scan, scheduled_bin_cleanup, bin_cleanup_file_age, job_archive_age ) values ( (select nextval('SETTINGS_ID_SEQ')), '/home/ddp/src/photoassistant/', 'images_to_process/#new_img_dir/', 'photos/', '.pa_bin/', 1, 1, '0.55', 1, 1, 7, 30, 3 ); +insert into SETTINGS ( id, base_path, import_path, storage_path, recycle_bin_path, default_refimg_model, default_scan_model, default_threshold, scheduled_import_scan, scheduled_storage_scan, scheduled_bin_cleanup, bin_cleanup_file_age, job_archive_age ) values ( (select nextval('SETTINGS_ID_SEQ')), '/home/ddp/src/photoassistant/', 'images_to_process/', 'photos/', '.pa_bin/', 1, 1, '0.55', 1, 1, 7, 30, 3 ); -- DEV(cam): --insert into SETTINGS ( id, base_path, import_path, storage_path, recycle_bin_path, default_refimg_model, default_scan_model, default_threshold, scheduled_import_scan, scheduled_storage_scan, scheduled_bin_cleanup, bin_cleanup_file_age, job_archive_age ) values ( (select nextval('SETTINGS_ID_SEQ')), 'c:/Users/cam/Desktop/code/python/photoassistant/', 'c:\images_to_process', 'photos/', '.pa_bin/', 1, 1, '0.55', 1, 1, 7, 30, 3 ); -- PROD: