Photo upload rework (#1042)

Partially reworks saving `GamePhoto`s in DB by doing the following:
- The DB method now takes interface-type parameters instead of
`SerializedPhoto` (making it work more similarly to other similar
methods), and this PR moves boundary parsing logic from
`SerializedPhotoSubject` to `PhotoHelper`, in order to start untangling
photo-related game endpoint classes from the DB project
- The level the photo is supposed to be attributed to is now passed as a
parameter to the DB method
- The DB method now returns the newly created `GamePhoto`
- levels of photos created from reports will now retain their original
level ID and slot type, even if it's not a user/developer level (e.g.
pod), or if it doesn't exist on the server

Author: Toastbrot236

Refresh.Database/GameDatabaseContext.Photos.cs

@@ -3,6 +3,8 @@
 using Refresh.Database.Models.Users;
 using Refresh.Database.Models.Levels;
 using Refresh.Database.Models.Photos;
+using Refresh.Database.Query;
+using Refresh.Database.Helpers;
 
 namespace Refresh.Database;
 
@@ -26,7 +28,7 @@ public partial class GameDatabaseContext // Photos
         .Include(p => p.Subject4User)
         .Include(p => p.Subject4User!.Statistics);
 
-    public void UploadPhoto(SerializedPhoto photo, GameUser publisher)
+    public GamePhoto UploadPhoto(IPhotoUpload photo, IEnumerable<IPhotoUploadSubject> subjects, GameUser publisher, GameLevel? level)
     {
         GamePhoto newPhoto = new()
         {
@@ -36,33 +38,24 @@ public void UploadPhoto(SerializedPhoto photo, GameUser publisher)
             PlanHash = photo.PlanHash,
 
             Publisher = publisher,
-            LevelType = photo.Level?.Type ?? "",
-            OriginalLevelId = photo.Level?.LevelId ?? 0,
-            OriginalLevelName = photo.Level?.Title ?? "",
+            Level = level,
+            LevelType = photo.LevelType ?? "",
+            OriginalLevelId = photo.LevelId ?? 0,
+            OriginalLevelName = photo.LevelTitle ?? "",
 
             TakenAt = DateTimeOffset.FromUnixTimeSeconds(Math.Clamp(photo.Timestamp, this._time.EarliestDate, this._time.TimestampSeconds)),
             PublishedAt = this._time.Now,
         };
 
-        GameLevel? level = null;
-
-        if (photo.Level?.Type is "user" or "developer") 
-        {
-            level = this.GetLevelByIdAndType(photo.Level.Type, photo.Level.LevelId);
-            newPhoto.Level = level;
-        }
-
-        float[] bounds = new float[SerializedPhotoSubject.FloatCount];
-
-        List<GamePhotoSubject> gameSubjects = new(photo.PhotoSubjects.Count);
-        foreach (SerializedPhotoSubject subject in photo.PhotoSubjects)
+        List<GamePhotoSubject> gameSubjects = new(subjects.Count());
+        foreach (IPhotoUploadSubject subject in subjects)
         {
             GameUser? subjectUser = null;
 
             if (!string.IsNullOrEmpty(subject.Username)) 
                 subjectUser = this.GetUserByUsername(subject.Username);
 
-            SerializedPhotoSubject.ParseBoundsList(subject.BoundsList, bounds);
+            float[] bounds = PhotoHelper.ParseBoundsList(subject.BoundsList);
 
             gameSubjects.Add(new GamePhotoSubject(subjectUser, subject.DisplayName, bounds));
 
@@ -96,6 +89,7 @@ public void UploadPhoto(SerializedPhoto photo, GameUser publisher)
         }
 
         this.CreatePhotoUploadEvent(publisher, newPhoto);
+        return newPhoto;
     }
 
     public void RemovePhoto(GamePhoto photo)

Refresh.Database/Helpers/PhotoHelper.cs

@@ -0,0 +1,32 @@
+using System.Globalization;
+
+namespace Refresh.Database.Helpers;
+
+public abstract class PhotoHelper
+{
+    public const byte SubjectBoundaryCount = 4;
+    
+    public static float[] ParseBoundsList(string input)
+    {
+        if (string.IsNullOrWhiteSpace(input))
+            throw new ArgumentException($"Bounds are empty");
+
+        string[] boundsStr = input.Split(',');
+        if (boundsStr.Length != SubjectBoundaryCount)
+            throw new ArgumentException($"Number of bounds is invalid ({boundsStr.Length} instead of {SubjectBoundaryCount})");
+
+        float[] boundsParsed = new float[SubjectBoundaryCount];
+        for (int i = 0; i < SubjectBoundaryCount; i++)
+        {
+            // No OutOfRangeExceptions as both arrays are ensured to be as long as the constant
+            string boundaryStr = boundsStr[i];
+
+            if (!float.TryParse(boundaryStr, NumberFormatInfo.InvariantInfo, out float f))
+                throw new FormatException($"Boundary {boundaryStr} ({i+1}/{SubjectBoundaryCount}) is not a float");
+
+            boundsParsed[i] = f;
+        }
+
+        return boundsParsed;
+    }
+}

Refresh.Database/Models/Photos/SerializedPhoto.cs

@@ -1,12 +1,13 @@
 using System.Xml.Serialization;
+using Refresh.Database.Query;
 
 namespace Refresh.Database.Models.Photos;
 
 #nullable disable
 
 [XmlRoot("photo")]
 [XmlType("photo")]
-public class SerializedPhoto
+public class SerializedPhoto : IPhotoUpload
 {
     [XmlAttribute("timestamp")]
     public long Timestamp { get; set; }
@@ -21,8 +22,16 @@ public class SerializedPhoto
     [XmlElement("medium")] public string MediumHash { get; set; }
     [XmlElement("large")] public string LargeHash { get; set; }
     [XmlElement("plan")] public string PlanHash { get; set; }
+
+    #nullable restore
 
-    [XmlElement("slot")] public SerializedPhotoLevel Level { get; set; }
+    /// <remarks>
+    /// Should never be null, but marked as nullable anyway in order to prevent another bad implementation from breaking
+    /// </remarks>
+    [XmlElement("slot")] public SerializedPhotoLevel? Level { get; set; }
+    [XmlIgnore] public int? LevelId => this.Level?.LevelId;
+    [XmlIgnore] public string? LevelType => this.Level?.Type;
+    [XmlIgnore] public string? LevelTitle => this.Level?.Title;
 
-    [XmlArray("subjects")] public List<SerializedPhotoSubject> PhotoSubjects { get; set; }
+    [XmlArray("subjects")] public List<SerializedPhotoSubject> PhotoSubjects { get; set; } = [];
 }

Refresh.Database/Models/Photos/SerializedPhotoSubject.cs

@@ -1,13 +1,13 @@
-using System.Globalization;
 using System.Xml.Serialization;
+using Refresh.Database.Query;
 
 namespace Refresh.Database.Models.Photos;
 
 #nullable disable
 
 [XmlRoot("subject")]
 [XmlType("subject")]
-public class SerializedPhotoSubject
+public class SerializedPhotoSubject : IPhotoUploadSubject
 {
     [XmlElement("npHandle")]
     public string Username { get; set; }
@@ -17,31 +17,4 @@ public class SerializedPhotoSubject
 
     [XmlElement("bounds")]
     public string BoundsList { get; set; }
-    
-    public const byte FloatCount = 4;
-    
-    public static void ParseBoundsList(ReadOnlySpan<char> input, float[] floats)
-    {
-        byte start = 0;
-        byte floatIndex = 0;
-
-        for (byte i = 0; i < input.Length; i++)
-        {
-            if(floatIndex == FloatCount - 1) break; // won't catch the last float - not worth reading
-            if (input[i] != ',') continue;
-            
-            if (!float.TryParse(input.Slice(start, i - start), NumberFormatInfo.InvariantInfo, out float f))
-                throw new FormatException("Invalid format");
-                
-            floats[floatIndex] = f;
-            floatIndex++;
-            start = (byte)(i + 1);
-        }
-        
-        // parse the last float value - bounds does not end with comma
-        if (!float.TryParse(input[start..], NumberFormatInfo.InvariantInfo, out float lastFloat))
-            throw new FormatException("Invalid format");
-
-        floats[FloatCount - 1] = lastFloat;
-    }
 }

Refresh.Database/Query/IPhotoUpload.cs

@@ -0,0 +1,13 @@
+namespace Refresh.Database.Query;
+
+public interface IPhotoUpload
+{
+    int? LevelId { get; }
+    string? LevelType { get; }
+    string? LevelTitle { get; }
+    string SmallHash { get; }
+    string MediumHash { get;}
+    string LargeHash { get; }
+    string PlanHash { get; }
+    long Timestamp { get; }
+}

Refresh.Database/Query/IPhotoUploadSubject.cs

@@ -0,0 +1,8 @@
+namespace Refresh.Database.Query;
+
+public interface IPhotoUploadSubject
+{
+    string Username { get; }
+    string DisplayName { get; }
+    string BoundsList { get; }
+}

Refresh.Interfaces.Game/Endpoints/PhotoEndpoints.cs

@@ -58,8 +58,9 @@ public Response UploadPhoto(RequestContext context, SerializedPhoto body, GameDa
                 return Unauthorized;
         }
 
-        database.UploadPhoto(body, user);
+        GameLevel? level = body.Level == null ? null : database.GetLevelByIdAndType(body.Level.Type, body.Level.LevelId);
 
+        database.UploadPhoto(body, body.PhotoSubjects, user, level);
         return OK;
     }
 

Refresh.Interfaces.Game/Endpoints/ReportingEndpoints.cs

@@ -53,12 +53,7 @@ public Response UploadReport(RequestContext context, GameDatabaseContext databas
         }
 
         string jpegHash = body.JpegHash;
-        
-        //If the level is specified but its invalid, set it to 0, to indicate the level is unknown
-        //This case is hit when someone makes a grief report from a non-existent level, which we allow
-        if (body.LevelId != 0 && level == null)
-            body.LevelId = 0;
-        
+
         //Basic validation
         if (body.Players is { Length: > 4 } || body.ScreenElements is { Player.Length: > 4 })
             // Return OK on PSP, since if we dont, it will error when trying to access the community moon and soft-lock the save file
@@ -84,18 +79,14 @@ public Response UploadReport(RequestContext context, GameDatabaseContext databas
                 MediumHash = jpegHash,
                 LargeHash = jpegHash,
                 PlanHash = "0",
-                Level = body.LevelId == 0 || level == null ? null : new SerializedPhotoLevel
+                Level = new SerializedPhotoLevel
                 {
-                    LevelId = level.LevelId,
-                    Title = level.Title,
-                    Type = level.SlotType switch {
-                        GameSlotType.User => "user",
-                        GameSlotType.Story => "developer",
-                        _ => throw new ArgumentOutOfRangeException(),
-                    },
+                    LevelId = body.LevelId,
+                    Title = level?.Title ?? "",
+                    Type = body.LevelType
                 },
                 PhotoSubjects = subjects,
-            }, user);
+            }, subjects, user, level);
 
             return OK; // just upload photo
         }

RefreshTests.GameServer/TestContext.cs

@@ -16,6 +16,7 @@
 using Refresh.Interfaces.Game.Endpoints.DataTypes.Request;
 using Refresh.Database.Models.Playlists;
 using Refresh.Interfaces.APIv3.Endpoints.DataTypes.Request;
+using Refresh.Database.Models.Photos;
 
 namespace RefreshTests.GameServer;
 
@@ -144,7 +145,7 @@ public GameLevel CreateLevel(GameUser author, string title, string description,
         return level;
     }
 
-    public void CreatePhotoWithSubject(GameUser author, string imageHash)
+    public GamePhoto CreatePhotoWithSubject(GameUser author, string imageHash, GameLevel? level = null)
     {
         // TODO: Return newly created GamePhoto
         if (this.Database.GetAssetFromHash(imageHash) == null)
@@ -155,22 +156,25 @@ public void CreatePhotoWithSubject(GameUser author, string imageHash)
             });
         }
 
-        this.Database.UploadPhoto(new()
+        SerializedPhoto photo = new()
         {
             AuthorName = author.Username,
             SmallHash = imageHash,
             MediumHash = imageHash,
             LargeHash = imageHash,
-            PhotoSubjects = 
-            [
-                new()
-                {
-                    Username = author.Username,
-                    DisplayName = author.Username,
-                    BoundsList = "10,10,10,10"
-                }
-            ]
-        }, author);
+            
+        };
+        IEnumerable<SerializedPhotoSubject> subjects = 
+        [
+            new()
+            {
+                Username = author.Username,
+                DisplayName = author.Username,
+                BoundsList = "10,10,10,10"
+            }
+        ];
+
+        return this.Database.UploadPhoto(photo, subjects, author, level);
     }
 
     public void FillLeaderboard(GameLevel level, int count, byte type)

RefreshTests.GameServer/Tests/Levels/LevelDeletionTests.cs

@@ -73,7 +73,7 @@ public void DeletesLevelWithPhotos()
             ],
         };
 
-        context.Database.UploadPhoto(photo, author);
+        context.Database.UploadPhoto(photo, photo.PhotoSubjects, author, level);
         context.Database.Refresh();
 
         level = context.Database.GetLevelById(level.LevelId);