Get files GUID of the current application instance

We store files by application instance so the same file will have one
row per AI for example in the case of LTI upgrades.

We use GUID as a "source of truth" in other tables like assignment and
in general for any communication with the H service (generating group
and user IDs).

lms/services/file.py

@@ -2,7 +2,7 @@
 
 from sqlalchemy import func
 
-from lms.models import File
+from lms.models import ApplicationInstance, File
 from lms.services.upsert import bulk_upsert
 
 
@@ -13,32 +13,39 @@ def __init__(self, application_instance, db):
 
     def get(self, lms_id, type_, course_id=None) -> Optional[File]:
         """Return the file with the given parameters or None."""
-        return self._file_search_query(
-            application_instance=self._application_instance,
-            lms_id=lms_id,
-            type_=type_,
-            course_id=course_id,
-        ).one_or_none()
+        return (
+            self._file_search_query(
+                guid=self._application_instance.tool_consumer_instance_guid,
+                lms_id=lms_id,
+                type_=type_,
+                course_id=course_id,
+            )
+            # We might have recorded the same file twice in different ApplicationInstances
+            # as we query by GUID we might get more than once row, in that case we prefer the newest
+            .order_by(File.id.desc()).first()
+        )
 
     def find_copied_file(self, new_course_id, original_file: File):
         """Find an equivalent file to `original_file` in our DB."""
         return (
             self._file_search_query(
-                # Both files should be in the same application instance
-                application_instance=original_file.application_instance,
+                # Both files should be in the same institution
+                guid=self._application_instance.tool_consumer_instance_guid,
                 # Files of the same type
                 type_=original_file.type,
                 # Looking for files in the new course only
                 course_id=new_course_id,
                 # And as a heuristic, we reckon same name, same size, probably the same file
                 name=original_file.name,
                 size=original_file.size,
-            ).filter(
+            )
+            .filter(
                 # We don't want to find the same file we are looking for
                 File.id
                 != original_file.id,
             )
-            # We might find more than one matching file, take any of them
+            # We might find more than one matching file, prefer the newest
+            .order_by(File.id.desc())
             .first()
         )
 
@@ -57,32 +64,30 @@ def upsert(self, file_dicts):
         )
 
     def _file_search_query(
-        self,
-        application_instance,
-        type_,
-        *,
-        lms_id=None,
-        course_id=None,
-        name=None,
-        size=None
+        self, guid, type_, *, lms_id=None, course_id=None, name=None, size=None
     ):
         """Return a `File` query with the passed parameters applied as filters."""
-        query = self._db.query(File).filter_by(
-            application_instance_id=application_instance.id,
-            type=type_,
+        query = (
+            self._db.query(File)
+            .join(ApplicationInstance)
+            .filter(
+                # We don't query by application_instance but any file belonging to an AI with the same GUID
+                ApplicationInstance.tool_consumer_instance_guid == guid,
+                File.type == type_,
+            )
         )
 
         if lms_id:
-            query = query.filter_by(lms_id=lms_id)
+            query = query.filter(File.lms_id == lms_id)
 
         if course_id:
-            query = query.filter_by(course_id=course_id)
+            query = query.filter(File.course_id == course_id)
 
         if name:
-            query = query.filter_by(name=name)
+            query = query.filter(File.name == name)
 
         if size:
-            query = query.filter_by(size=size)
+            query = query.filter(File.size == size)
 
         return query
 

tests/unit/lms/services/file_test.py

@@ -15,13 +15,29 @@ def test_get_returns_the_matching_file_if_there_is_one(
 
         assert svc.get(file_.lms_id, file_.type) == file_
 
+    def test_get_matching_guid(self, svc, db_session, application_instance):
+        same_guid_ai = factories.ApplicationInstance(
+            tool_consumer_instance_guid=application_instance.tool_consumer_instance_guid
+        )
+        file_ = factories.File(application_instance=same_guid_ai)
+        db_session.flush()
+
+        # Assert we an in fact querying by other AI
+        # pylint: disable=protected-access
+        assert svc._application_instance != same_guid_ai
+        assert svc.get(file_.lms_id, file_.type) == file_
+
     def test_get_returns_None_if_theres_no_matching_file(self, svc):
         assert not svc.get("unknown_file_id", "canvas_file")
 
-    def test_get_doesnt_return_matching_files_from_other_application_instances(
-        self, svc
+    def test_get_doesnt_return_matching_files_from_application_instances_with_different_guid(
+        self, svc, db_session
     ):
-        file_ = factories.File()
+        application_instance = factories.ApplicationInstance(
+            tool_consumer_instance_guid="OTHER"
+        )
+        file_ = factories.File(application_instance=application_instance)
+        db_session.flush()
 
         assert not svc.get(file_.lms_id, file_.type)