fix: Better handling of invalid CF cache entries

Due to changes in v4.1.1, sometimes invalid cache entries (zero-value)
were written to the cache. These are now treated as invalid and matches
by name will be performed.

Fixes #160
pull/201/head
Robert Dailey 1 year ago
parent 18edb84133
commit 8c7768891e

@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
### Fixed
- More scenarios were causing custom formats to sometimes not be synced (#160).
## [4.1.2] - 2023-01-06
### Fixed

@ -1,6 +1,5 @@
using Newtonsoft.Json.Linq;
using Recyclarr.TrashLib.Services.CustomFormat.Models;
using Recyclarr.TrashLib.Services.CustomFormat.Models.Cache;
namespace Recyclarr.TrashLib.TestLibrary;
@ -8,39 +7,45 @@ public static class NewCf
{
public static CustomFormatData Data(string name, string trashId, int? score = null)
{
var json = JObject.Parse($"{{'name':'{name}'}}");
return new CustomFormatData("", name, trashId, score, new JObject(json));
return Data(name, trashId, score, JObject.Parse($"{{'name':'{name}'}}"));
}
public static ProcessedCustomFormatData Processed(string name, string trashId, int? score = null)
public static CustomFormatData Data(string name, string trashId, int? score, JObject json)
{
return new ProcessedCustomFormatData(Data(name, trashId, score));
return new CustomFormatData("", name, trashId, score, json);
}
public static ProcessedCustomFormatData Processed(string name, string trashId, int? score, JObject json)
public static ProcessedCustomFormatData ProcessedWithScore(string name, string trashId, int score, JObject json)
{
return new ProcessedCustomFormatData(new CustomFormatData("", name, trashId, score, json));
return new ProcessedCustomFormatData(Data(name, trashId, score, json));
}
public static ProcessedCustomFormatData ProcessedWithScore(string name, string trashId, int score, int formatId = 0)
{
return new ProcessedCustomFormatData(Data(name, trashId, score))
{
FormatId = formatId
};
}
public static ProcessedCustomFormatData Processed(string name, string trashId, JObject json)
{
return Processed(name, trashId, null, json);
return Processed(name, trashId, 0, json);
}
public static ProcessedCustomFormatData Processed(string name, string trashId, TrashIdMapping cacheEntry)
public static ProcessedCustomFormatData Processed(string name, string trashId, int formatId = 0)
{
return new ProcessedCustomFormatData(Data(name, trashId))
{
CacheEntry = cacheEntry
FormatId = formatId
};
}
public static ProcessedCustomFormatData Processed(string name, string trashId, JObject json,
TrashIdMapping? cacheEntry)
public static ProcessedCustomFormatData Processed(string name, string trashId, int formatId, JObject json)
{
return new ProcessedCustomFormatData(new CustomFormatData("", name, trashId, null, json))
return new ProcessedCustomFormatData(Data(name, trashId, null, json))
{
CacheEntry = cacheEntry
FormatId = formatId
};
}
}

@ -6,7 +6,6 @@ using Recyclarr.TrashLib.Cache;
using Recyclarr.TrashLib.Services.CustomFormat;
using Recyclarr.TrashLib.Services.CustomFormat.Models;
using Recyclarr.TrashLib.Services.CustomFormat.Models.Cache;
using Recyclarr.TrashLib.Services.CustomFormat.Processors.PersistenceSteps;
using Recyclarr.TrashLib.TestLibrary;
using Serilog;
@ -106,31 +105,48 @@ public class CachePersisterTest
var ctx = new Context();
// Load initial CfCache just to test that it gets replaced
var testCfObj = new CustomFormatCache
ctx.ServiceCache.Load<CustomFormatCache>().Returns(new CustomFormatCache
{
TrashIdMappings = new Collection<TrashIdMapping> {new("", "") {CustomFormatId = 5}}
};
ctx.ServiceCache.Load<CustomFormatCache>().Returns(testCfObj);
TrashIdMappings = new Collection<TrashIdMapping> {new("trashid", "", 1)}
});
ctx.Persister.Load();
// Update with new cached items
var results = new CustomFormatTransactionData();
results.NewCustomFormats.Add(NewCf.Processed("cfname", "trashid", 10));
var customFormatData = new List<ProcessedCustomFormatData>
{
new(NewCf.Data("", "trashid")) {CacheEntry = new TrashIdMapping("trashid", "", 10)}
NewCf.Processed("trashid", "name", 5)
};
ctx.Persister.Update(customFormatData);
ctx.Persister.CfCache.Should().BeEquivalentTo(new CustomFormatCache
{
TrashIdMappings = new Collection<TrashIdMapping> {customFormatData[0].CacheEntry!}
TrashIdMappings = new Collection<TrashIdMapping>
{
new(customFormatData[0].TrashId, customFormatData[0].Name, customFormatData[0].FormatId)
}
});
}
customFormatData.Should().ContainSingle()
.Which.CacheEntry.Should().BeEquivalentTo(
new TrashIdMapping("trashid", "") {CustomFormatId = 10});
[Test]
public void Saving_skips_custom_formats_with_zero_id()
{
var ctx = new Context();
// Update with new cached items
var customFormatData = new List<ProcessedCustomFormatData>
{
NewCf.Processed("trashid1", "name", 5),
NewCf.Processed("trashid2", "invalid")
};
ctx.Persister.Update(customFormatData);
ctx.Persister.CfCache.Should().BeEquivalentTo(new CustomFormatCache
{
TrashIdMappings = new Collection<TrashIdMapping>
{
new(customFormatData[0].TrashId, customFormatData[0].Name, customFormatData[0].FormatId)
}
});
}
[Test]

@ -99,9 +99,9 @@ public class GuideProcessorTest
var expectedProcessedCustomFormatData = new List<ProcessedCustomFormatData>
{
NewCf.Processed("Surround Sound", "43bb5f09c79641e7a22e48d440bd8868", 500,
NewCf.ProcessedWithScore("Surround Sound", "43bb5f09c79641e7a22e48d440bd8868", 500,
ctx.ReadJson("ImportableCustomFormat1_Processed.json")),
NewCf.Processed("DTS-HD/DTS:X", "4eb3c272d48db8ab43c2c85283b69744", 480,
NewCf.ProcessedWithScore("DTS-HD/DTS:X", "4eb3c272d48db8ab43c2c85283b69744", 480,
ctx.ReadJson("ImportableCustomFormat2_Processed.json")),
NewCf.Processed("No Score", "abc")
};

@ -82,13 +82,13 @@ public class CustomFormatStepTest
var testCache = new CustomFormatCache
{
TrashIdMappings = new Collection<TrashIdMapping> {new("id1000", "")}
TrashIdMappings = new Collection<TrashIdMapping> {new("id1000", "", 1)}
};
processor.Process(guideData, testConfig, testCache);
processor.DeletedCustomFormatsInCache.Should()
.BeEquivalentTo(new[] {new TrashIdMapping("id1000", "")});
.BeEquivalentTo(new[] {new TrashIdMapping("id1000", "", 1)});
processor.ProcessedCustomFormats.Should().BeEquivalentTo(new List<ProcessedCustomFormatData>
{
NewCf.Processed("name1", "id1")
@ -152,35 +152,4 @@ public class CustomFormatStepTest
processor.DeletedCustomFormatsInCache.Should().BeEmpty();
processor.ProcessedCustomFormats.Should().BeEmpty();
}
[Test, AutoMockData]
public void Score_from_json_takes_precedence_over_score_from_guide(CustomFormatStep processor)
{
var guideData = new List<CustomFormatData>
{
NewCf.Data("name1", "id1", 100)
};
var testConfig = new List<CustomFormatConfig>
{
new()
{
TrashIds = new List<string> {"id1"},
QualityProfiles = new List<QualityProfileScoreConfig>
{
new() {Name = "profile", Score = 200}
}
}
};
processor.Process(guideData, testConfig, null);
processor.DeletedCustomFormatsInCache.Should().BeEmpty();
processor.ProcessedCustomFormats.Should()
.BeEquivalentTo(new List<ProcessedCustomFormatData>
{
NewCf.Processed("name1", "id1", 100)
},
op => op.Using(new JsonEquivalencyStep()));
}
}

@ -74,7 +74,7 @@ public class QualityProfileStepTest
{
CustomFormats = new List<ProcessedCustomFormatData>
{
NewCf.Processed("", "id1", 100)
NewCf.ProcessedWithScore("", "id1", 100)
},
QualityProfiles = new List<QualityProfileScoreConfig>
{
@ -109,7 +109,7 @@ public class QualityProfileStepTest
{
CustomFormats = new List<ProcessedCustomFormatData>
{
NewCf.Processed("name1", "id1", 0)
NewCf.ProcessedWithScore("name1", "id1", 0)
},
QualityProfiles = new List<QualityProfileScoreConfig>
{

@ -14,7 +14,7 @@ public class CustomFormatApiPersistenceStepTest
{
private static ProcessedCustomFormatData QuickMakeCf(string cfName, string trashId, int cfId)
{
return NewCf.Processed(cfName, trashId, new TrashIdMapping(trashId, cfName) {CustomFormatId = cfId});
return NewCf.Processed(cfName, trashId, cfId);
}
[Test]
@ -24,7 +24,7 @@ public class CustomFormatApiPersistenceStepTest
transactions.NewCustomFormats.Add(QuickMakeCf("cfname1", "trashid1", 1));
transactions.UpdatedCustomFormats.Add(QuickMakeCf("cfname2", "trashid2", 2));
transactions.UnchangedCustomFormats.Add(QuickMakeCf("cfname3", "trashid3", 3));
transactions.DeletedCustomFormatIds.Add(new TrashIdMapping("trashid4", "cfname4") {CustomFormatId = 4});
transactions.DeletedCustomFormatIds.Add(new TrashIdMapping("trashid4", "cfname4", 4));
var api = Substitute.For<ICustomFormatService>();

@ -122,8 +122,8 @@ public class JsonTransactionStepTest
var guideCfs = new List<ProcessedCustomFormatData>
{
NewCf.Processed("created", "id1", guideCfData[0]),
NewCf.Processed("updated_different_name", "id2", guideCfData[1], new TrashIdMapping("id2", "", 2)),
NewCf.Processed("no_change", "id3", guideCfData[2], new TrashIdMapping("id3", "", 3))
NewCf.Processed("updated_different_name", "id2", 2, guideCfData[1]),
NewCf.Processed("no_change", "id3", 3, guideCfData[2])
};
processor.Process(guideCfs, radarrCfs);
@ -231,12 +231,12 @@ public class JsonTransactionStepTest
}");
var deletedCfsInCache = new List<TrashIdMapping>
{
new("", "") {CustomFormatId = 2}
new("", "", 1) {CustomFormatId = 2}
};
var guideCfs = new List<ProcessedCustomFormatData>
{
NewCf.Processed("updated", "", guideCfData, new TrashIdMapping("", "") {CustomFormatId = 1})
NewCf.Processed("updated", "", 1, guideCfData)
};
var radarrCfs = JsonConvert.DeserializeObject<List<JObject>>(radarrCfData);
@ -324,7 +324,7 @@ public class JsonTransactionStepTest
var guideCfs = new List<ProcessedCustomFormatData>
{
NewCf.Processed("first", "", new TrashIdMapping("", "first", 2))
NewCf.Processed("first", "", 2)
};
processor.Process(guideCfs, serviceCfs);
@ -353,7 +353,7 @@ public class JsonTransactionStepTest
processor.Process(guideCfs, serviceCfs);
processor.Transactions.UpdatedCustomFormats.Should().BeEquivalentTo(
new[] {NewCf.Processed("first", "", new TrashIdMapping("", "first", 1))},
o => o.Including(x => x.CacheEntry!.CustomFormatId));
new[] {NewCf.Processed("first", "", 1)},
o => o.Including(x => x.FormatId));
}
}

@ -7,7 +7,6 @@ using NUnit.Framework;
using Recyclarr.TestLibrary.NSubstitute;
using Recyclarr.TrashLib.Services.CustomFormat.Api;
using Recyclarr.TrashLib.Services.CustomFormat.Models;
using Recyclarr.TrashLib.Services.CustomFormat.Models.Cache;
using Recyclarr.TrashLib.Services.CustomFormat.Processors.PersistenceSteps;
using Recyclarr.TrashLib.TestLibrary;
@ -47,8 +46,7 @@ public class QualityProfileApiPersistenceStepTest
var cfScores = new Dictionary<string, QualityProfileCustomFormatScoreMapping>
{
{
"profile1", CfTestUtils.NewMapping(new FormatMappingEntry(
NewCf.Processed("", "", new TrashIdMapping("", "") {CustomFormatId = 4}), 100))
"profile1", CfTestUtils.NewMapping(new FormatMappingEntry(NewCf.Processed("", "", 4), 100))
}
};
@ -109,7 +107,7 @@ public class QualityProfileApiPersistenceStepTest
{
{
"profile1", CfTestUtils.NewMappingWithReset(
new FormatMappingEntry(NewCf.Processed("", "", new TrashIdMapping("", "", 2)), 100))
new FormatMappingEntry(NewCf.Processed("", "", 2), 100))
}
};
@ -183,11 +181,11 @@ public class QualityProfileApiPersistenceStepTest
{
"profile1", CfTestUtils.NewMapping(
// First match by ID
new FormatMappingEntry(NewCf.Processed("", "", new TrashIdMapping("", "", 4)), 100),
new FormatMappingEntry(NewCf.Processed("", "", 4), 100),
// Should NOT match because we do not use names to assign scores
new FormatMappingEntry(NewCf.Processed("", "", new TrashIdMapping("", "")), 101),
new FormatMappingEntry(NewCf.Processed("", ""), 101),
// Second match by ID
new FormatMappingEntry(NewCf.Processed("", "", new TrashIdMapping("", "", 1)), 102))
new FormatMappingEntry(NewCf.Processed("", "", 1), 102))
}
};

@ -28,13 +28,13 @@ internal class CustomFormatService : ICustomFormatService
if (response != null)
{
cf.SetCache(response.Value<int>("id"));
cf.FormatId = response.Value<int>("id");
}
}
public async Task UpdateCustomFormat(ProcessedCustomFormatData cf)
{
await _service.Request("customformat", cf.GetCustomFormatId())
await _service.Request("customformat", cf.FormatId)
.PutJsonAsync(cf.Json)
.ReceiveJson<JObject>();
}

@ -56,10 +56,12 @@ public class CachePersister : ICachePersister
public void Update(IEnumerable<ProcessedCustomFormatData> customFormats)
{
Log.Debug("Updating cache");
CfCache = new CustomFormatCache();
CfCache!.TrashIdMappings.AddRange(customFormats
.Where(cf => cf.CacheEntry != null)
.Select(cf => cf.CacheEntry!));
.Where(cf => cf.FormatId is not 0)
.Select(cf => new TrashIdMapping(cf.TrashId, cf.Name, cf.FormatId)));
Log.Debug("Updated Cache with {Mappings}", CfCache.TrashIdMappings.Select(
x => $"TrashID: {x.TrashId}, Name: {x.CustomFormatName}, FormatID: {x.CustomFormatId}"));
}
}

@ -4,7 +4,7 @@ using Recyclarr.TrashLib.Cache;
namespace Recyclarr.TrashLib.Services.CustomFormat.Models.Cache;
[CacheObjectName("custom-format-cache")]
public class CustomFormatCache
public record CustomFormatCache
{
public const int LatestVersion = 1;
@ -12,16 +12,4 @@ public class CustomFormatCache
public Collection<TrashIdMapping> TrashIdMappings { get; init; } = new();
}
public class TrashIdMapping
{
public TrashIdMapping(string trashId, string customFormatName, int customFormatId = default)
{
TrashId = trashId;
CustomFormatName = customFormatName;
CustomFormatId = customFormatId;
}
public string TrashId { get; }
public string CustomFormatName { get; }
public int CustomFormatId { get; set; }
}
public record TrashIdMapping(string TrashId, string CustomFormatName, int CustomFormatId);

@ -1,6 +1,4 @@
using System.Diagnostics.CodeAnalysis;
using Newtonsoft.Json.Linq;
using Recyclarr.TrashLib.Services.CustomFormat.Models.Cache;
namespace Recyclarr.TrashLib.Services.CustomFormat.Models;
@ -18,17 +16,5 @@ public class ProcessedCustomFormatData
public string TrashId => _data.TrashId;
public int? Score => _data.Score;
public JObject Json { get; set; }
public TrashIdMapping? CacheEntry { get; set; }
public void SetCache(int customFormatId)
{
// Do not pass the customFormatId to constructor since an instance may already exist.
CacheEntry ??= new TrashIdMapping(TrashId, Name);
CacheEntry.CustomFormatId = customFormatId;
}
[SuppressMessage("Microsoft.Design", "CA1024", Justification = "Method throws an exception")]
public int GetCustomFormatId()
=> CacheEntry?.CustomFormatId ??
throw new InvalidOperationException("CacheEntry must exist to obtain custom format ID");
public int FormatId { get; set; }
}

@ -24,7 +24,7 @@ public class CustomFormatStep : ICustomFormatStep
// For each ID listed under the `trash_ids` YML property, match it to an existing CF
_processedCustomFormats.AddRange(config
.SelectMany(c => c.TrashIds)
.Distinct(StringComparer.CurrentCultureIgnoreCase)
.Distinct(StringComparer.InvariantCultureIgnoreCase)
.Join(processedCfs,
id => id,
cf => cf.TrashId,
@ -38,9 +38,10 @@ public class CustomFormatStep : ICustomFormatStep
private static ProcessedCustomFormatData ProcessCustomFormatData(CustomFormatData cf,
CustomFormatCache? cache)
{
var map = cache?.TrashIdMappings.FirstOrDefault(c => c.TrashId == cf.TrashId);
return new ProcessedCustomFormatData(cf)
{
CacheEntry = cache?.TrashIdMappings.FirstOrDefault(c => c.TrashId == cf.TrashId)
FormatId = map?.CustomFormatId ?? 0
};
}
@ -51,11 +52,8 @@ public class CustomFormatStep : ICustomFormatStep
return;
}
static bool MatchCfInCache(ProcessedCustomFormatData cf, TrashIdMapping c)
=> cf.CacheEntry != null && cf.CacheEntry.TrashId == c.TrashId;
// Delete if CF is in cache and not in the guide or config
_deletedCustomFormatsInCache.AddRange(cache.TrashIdMappings
.Where(c => !ProcessedCustomFormats.Any(cf => MatchCfInCache(cf, c))));
_deletedCustomFormatsInCache.AddRange(
cache.TrashIdMappings.Where(map => ProcessedCustomFormats.All(cf => cf.TrashId != map.TrashId)));
}
}

@ -35,8 +35,8 @@ public class JsonTransactionStep : IJsonTransactionStep
continue;
}
// If cache entry is NOT null, that means we found the service by its ID
if (guideCf.CacheEntry is not null)
// If custom format ID is NOT zero, that means we found the serviceCf by its ID
if (guideCf.FormatId != 0)
{
// Check for conflicts with upstream CFs with the same name but different ID.
// If found, it is recorded and we skip this CF.
@ -45,15 +45,8 @@ public class JsonTransactionStep : IJsonTransactionStep
continue;
}
}
// Null cache entry use case
else
{
// Set the cache for use later (like updating scores) if it hasn't been updated already.
// This handles CFs that already exist in the service but aren't cached (they will be added to cache
// later).
guideCf.SetCache(serviceCf.Value<int>("id"));
}
guideCf.FormatId = serviceCf.Value<int>("id");
guideCf.Json = (JObject) serviceCf.DeepClone();
UpdateServiceCf(guideCf.Json, guideCfJson);
@ -73,7 +66,7 @@ public class JsonTransactionStep : IJsonTransactionStep
ProcessedCustomFormatData guideCf,
JObject serviceCf)
{
var conflictingServiceCf = FindServiceCf(serviceCfs, null, guideCf.Name);
var conflictingServiceCf = FindServiceCf(serviceCfs, 0, guideCf.Name);
if (conflictingServiceCf is null)
{
return false;
@ -103,15 +96,15 @@ public class JsonTransactionStep : IJsonTransactionStep
private static JObject? FindServiceCf(IReadOnlyCollection<JObject> serviceCfs, ProcessedCustomFormatData guideCf)
{
return FindServiceCf(serviceCfs, guideCf.CacheEntry?.CustomFormatId, guideCf.Name);
return FindServiceCf(serviceCfs, guideCf.FormatId, guideCf.Name);
}
private static JObject? FindServiceCf(IReadOnlyCollection<JObject> serviceCfs, int? cfId, string? cfName = null)
private static JObject? FindServiceCf(IReadOnlyCollection<JObject> serviceCfs, int cfId, string? cfName = null)
{
JObject? match = null;
// Try to find match in cache first
if (cfId is not null)
if (cfId is not 0)
{
match = serviceCfs.FirstOrDefault(rcf => cfId == rcf.Value<int>("id"));
}

@ -79,7 +79,6 @@ internal class QualityProfileApiPersistenceStep : IQualityProfileApiPersistenceS
QualityProfileCustomFormatScoreMapping scoreMap)
{
return scoreMap.Mapping.FirstOrDefault(
m => m.CustomFormat.CacheEntry != null &&
formatItem.Value<int>("format") == m.CustomFormat.CacheEntry.CustomFormatId);
m => formatItem.Value<int>("format") == m.CustomFormat.FormatId);
}
}

Loading…
Cancel
Save