Always refresh all expired

1 files changed, 58 insertions, 75 deletions

diff --git a/src/updater.rs b/src/updater.rs
index 9f6c7df..1aba08f 100644
--- a/src/updater.rs
+++ b/src/updater.rs
@@ -22,8 +22,7 @@ pub fn update_images(now: &DateTime<Utc>, db: &dyn Db, reg: &dyn Registries, fil
     let mut file_indices: Vec<usize> = vec![];
     let mut image_ranges: Vec<Range<usize>> = vec![];
     let mut imgs: Vec<DockerRef> = vec![];
-    let mut image_ids: Vec<i64> = vec![];
-    let mut expires_ats: Vec<DateTime<Utc>> = vec![];
+    let mut images: Vec<Image> = vec![];
 
     for (file_idx, file) in files.iter().enumerate() {
         for image in &file.images {
@@ -33,8 +32,8 @@ pub fn update_images(now: &DateTime<Utc>, db: &dyn Db, reg: &dyn Registries, fil
                 .unwrap_or("registry.hub.docker.com");
             let image_name = &file.content[img.image.clone()];
 
-            let (image_id, expires_at) = match db.get_image(registry, image_name) {
-                Some(img) => (img.id, img.expires_at),
+            let image_record = match db.get_image(registry, image_name) {
+                Some(x) => x,
                 None => {
                     let mut new_image = Image {
                         id: 0,
@@ -43,15 +42,51 @@ pub fn update_images(now: &DateTime<Utc>, db: &dyn Db, reg: &dyn Registries, fil
                         expires_at: DateTime::<Utc>::UNIX_EPOCH,
                     };
                     db.insert_image(&mut new_image);
-                    (new_image.id, new_image.expires_at)
+                    new_image
                 },
             };
 
             file_indices.push(file_idx);
             image_ranges.push(image.clone());
             imgs.push(img);
-            image_ids.push(image_id);
-            expires_ats.push(expires_at);
+            images.push(image_record);
+        }
+    }
+
+    // Cache refresh loop: refresh all expired images in the database
+    let mut image_errors: std::collections::HashMap<i64, String> = std::collections::HashMap::new();
+    let mut refreshed_images: std::collections::HashSet<i64> = std::collections::HashSet::new();
+    for expired in db.get_expired_images(now) {
+        let cache_max_age = reg.get_cache_ttl(&expired.registry);
+        let cached_tags = db.get_tags_sorted(expired.id);
+
+        match reg.get_tags(&expired.registry, &expired.image) {
+            Some(mut fetched_tags) => {
+                fetched_tags.sort();
+                let mut existing = cached_tags.iter().peekable();
+                for t in &fetched_tags {
+                    while existing.peek().is_some_and(|e| *e < t) {
+                        let gone = existing.next().unwrap();
+                        db.delete_tag(expired.id, gone);
+                    }
+                    if existing.peek() == Some(&t) {
+                        existing.next();
+                    } else {
+                        db.insert_tag(expired.id, t, now);
+                    }
+                }
+                for gone in existing {
+                    db.delete_tag(expired.id, gone);
+                }
+                let new_expires_at = *now + cache_max_age;
+                db.set_expires_at(expired.id, &new_expires_at);
+                refreshed_images.insert(expired.id);
+            }
+            None => {
+                let new_expires_at = *now + cache_max_age;
+                db.set_expires_at(expired.id, &new_expires_at);
+                image_errors.insert(expired.id, format!("image not found: {}", expired.image));
+            }
         }
     }
 
@@ -74,78 +109,24 @@ pub fn update_images(now: &DateTime<Utc>, db: &dyn Db, reg: &dyn Registries, fil
         let registry = img.registry.as_ref()
             .map(|x| &file.content[x.clone()])
             .unwrap_or("registry.hub.docker.com");
-        let cache_max_age = reg.get_cache_ttl(registry);
         let mut tag = img.tag.as_ref().map(|x| file.content[x.clone()].to_string());
-        let image_name = &file.content[img.image.clone()];
-        let image_id = image_ids[i];
-        let expires_at = expires_ats[i];
+        let image_record = &images[i];
+        let image_id = image_record.id;
 
         println!("Checking image {}", &file.content[image_ranges[i].clone()]);
         metrics::get().images_checked.with_label_values(&[registry]).inc();
 
-        let cache_stale = *now > expires_at;
-
-        let cached_tags: Vec<String> = db.get_tags_sorted(image_id);
-
-        let tags_result: Result<(Vec<String>, Option<String>), String> = if cache_stale {
-            println!("Invalid in cache");
-            match reg.get_tags(registry, image_name) {
-                Some(mut fetched_tags) => {
-                    fetched_tags.sort();
-                    let mut existing = cached_tags.iter().peekable();
-                    for t in &fetched_tags {
-                        while existing.peek().is_some_and(|e| *e < t) {
-                            let gone = existing.next().unwrap();
-                            db.delete_tag(image_id, gone);
-                        }
-                        if existing.peek() == Some(&t) {
-                            existing.next();
-                        } else {
-                            db.insert_tag(image_id, t, now);
-                        }
-                    }
-                    for gone in existing {
-                        db.delete_tag(image_id, gone);
-                    }
-                    let new_expires_at = *now + cache_max_age;
-                    db.set_expires_at(image_id, &new_expires_at);
-                    // @CLEANUP: Propagate the update to other instances. We should really avoid
-                    // this situation in the first place
-                    for j in (i + 1)..image_ids.len() {
-                        if image_ids[j] == image_id {
-                            expires_ats[j] = new_expires_at;
-                        }
-                    }
+        if let Some(err) = image_errors.get(&image_id) {
+            file_errors[file_idx] = Some(err.clone());
+            continue;
+        }
 
-                    Ok((fetched_tags, None))
-                }
-                None => {
-                    let new_expires_at = *now + cache_max_age;
-                    db.set_expires_at(image_id, &new_expires_at);
-                    // @CLEANUP: Propagate the update to other instances. We should really avoid
-                    // this situation in the first place
-                    for j in (i + 1)..image_ids.len() {
-                        if image_ids[j] == image_id {
-                            expires_ats[j] = new_expires_at;
-                        }
-                    }
-                    Err(format!("image not found: {}", image_name))
-                }
-            }
+        let tags = db.get_tags_sorted(image_id);
+        let tag_for_digest = tag.as_deref().unwrap_or("latest");
+        let cached_digest = if refreshed_images.contains(&image_id) {
+            None  // Cache was stale, ignore cached digest
         } else {
-            println!("Valid in cache");
-            let tag_for_digest = tag.as_deref().unwrap_or("latest");
-            let digest = db.get_tag_digest(image_id, tag_for_digest);
-
-            Ok((cached_tags, digest))
-        };
-
-        let (tags, cached_digest) = match tags_result {
-            Ok(v) => v,
-            Err(msg) => {
-                file_errors[file_idx] = Some(msg);
-                continue;
-            }
+            db.get_tag_digest(image_id, tag_for_digest)
         };
 
         if let Some(ref tag_str) = tag {
@@ -181,12 +162,12 @@ pub fn update_images(now: &DateTime<Utc>, db: &dyn Db, reg: &dyn Registries, fil
             let digest_result = match cached_digest {
                 Some(d) => Ok(d),
                 None => {
-                    match reg.get_digest(registry, image_name, tag_for_digest) {
+                    match reg.get_digest(registry, &image_record.image, tag_for_digest) {
                         Some(fetched) => {
                             db.update_tag_digest(image_id, tag_for_digest, &fetched, now);
                             Ok(fetched)
                         }
-                        None => Err(format!("digest not found for {}:{}", image_name, tag_for_digest)),
+                        None => Err(format!("digest not found for {}:{}", image_record.image, tag_for_digest)),
                     }
                 }
             };
@@ -631,6 +612,7 @@ mod tests {
         let reg = StubRegistry::default();
 
         reg.add_tag("registry.hub.docker.com", "redis", "6.0", "sha256:0000000000000000000000000000000000000000000000000000000000000001");
+        reg.add_tag("registry.hub.docker.com", "redis", "6.2", "sha256:0000000000000000000000000000000000000000000000000000000000000002");
 
         let files = vec![FileInput {
             path: "/test".into(),
@@ -642,7 +624,8 @@ mod tests {
 
         assert!(outcomes[0].is_err());
         assert!(db.get_image("registry.hub.docker.com", "redis").is_some());
-        assert!(db.get_tags_sorted(2).is_empty());
+        // Redis tags ARE populated (cache refresh happens before processing), but no patches are produced
+        assert_eq!(db.get_tags_sorted(2), vec!["6.0", "6.2"]);
     }
 
     #[test]