Cleanup attachment validation.

1 files changed, 82 insertions, 48 deletions

diff --git a/zenserver/cache/structuredcache.cpp b/zenserver/cache/structuredcache.cpp
index ec27265a7..feb8efb2e 100644
--- a/zenserver/cache/structuredcache.cpp
+++ b/zenserver/cache/structuredcache.cpp
@@ -48,6 +48,14 @@ ParseCachePolicy(const HttpServerRequest::QueryParams& QueryParams)
 	return QueryPolicy | StorePolicy | SkipPolicy;
 }
 
+struct AttachmentCount
+{
+	uint32_t New	 = 0;
+	uint32_t Valid	 = 0;
+	uint32_t Invalid = 0;
+	uint32_t Total	 = 0;
+};
+
 //////////////////////////////////////////////////////////////////////////
 
 HttpStructuredCacheService::HttpStructuredCacheService(ZenCacheStore&				  InCacheStore,
@@ -465,20 +473,20 @@ HttpStructuredCacheService::HandlePutCacheRecord(zen::HttpServerRequest& Request
 			TotalCount++;
 		});
 
-		const bool IsPartialCacheRecord = TotalCount != static_cast<int32_t>(ValidAttachments.size());
-
-		ZEN_DEBUG("PUT - '{}/{}' {} '{}', {} attachments {}",
+		ZEN_DEBUG("PUT - '{}/{}' {} '{}' attachments '{}/{}' (Valid/Total)",
 				  Ref.BucketSegment,
 				  Ref.HashKey,
 				  NiceBytes(Body.Size()),
 				  ToString(ContentType),
-				  ValidAttachments.size(),
-				  IsPartialCacheRecord ? "(PARTIAL)"sv : ""sv);
+				  TotalCount,
+				  ValidAttachments.size());
 
 		Body.SetContentType(ZenContentType::kCbObject);
 		m_CacheStore.Put(Ref.BucketSegment, Ref.HashKey, {.Value = Body});
 
-		if (StoreUpstream && !IsPartialCacheRecord)
+		const bool IsPartialRecord = TotalCount != static_cast<int32_t>(ValidAttachments.size());
+
+		if (StoreUpstream && !IsPartialRecord)
 		{
 			ZEN_ASSERT(m_UpstreamCache);
 			auto Result = m_UpstreamCache->EnqueueUpstream({.Type		= ZenContentType::kCbObject,
@@ -499,69 +507,68 @@ HttpStructuredCacheService::HandlePutCacheRecord(zen::HttpServerRequest& Request
 		}
 
 		CbObject			CacheRecord = Package.GetObject();
+		AttachmentCount		Count;
 		std::vector<IoHash> ValidAttachments;
-		int32_t				TotalCount	 = 0;
-		int32_t				NewCount	 = 0;
-		int32_t				InvalidCount = 0;
 
 		ValidAttachments.reserve(Package.GetAttachments().size());
 
-		CacheRecord.IterateAttachments(
-			[this, &Ref, &Package, &ValidAttachments, &TotalCount, &NewCount, &InvalidCount](CbFieldView HashView) {
-				const IoHash Hash = HashView.AsHash();
-				if (const CbAttachment* Attachment = Package.FindAttachment(Hash))
+		CacheRecord.IterateAttachments([this, &Ref, &Package, &ValidAttachments, &Count](CbFieldView HashView) {
+			const IoHash Hash = HashView.AsHash();
+			if (const CbAttachment* Attachment = Package.FindAttachment(Hash))
+			{
+				if (Attachment->IsCompressedBinary())
 				{
-					if (Attachment->IsCompressedBinary())
-					{
-						CompressedBuffer	   Chunk		= Attachment->AsCompressedBinary();
-						CidStore::InsertResult InsertResult = m_CidStore.AddChunk(Chunk);
+					CompressedBuffer	   Chunk		= Attachment->AsCompressedBinary();
+					CidStore::InsertResult InsertResult = m_CidStore.AddChunk(Chunk);
 
-						ValidAttachments.emplace_back(InsertResult.DecompressedId);
+					ValidAttachments.emplace_back(InsertResult.DecompressedId);
 
-						if (InsertResult.New)
-						{
-							NewCount++;
-						}
-					}
-					else
+					if (InsertResult.New)
 					{
-						ZEN_WARN("PUT - '{}/{}' '{}' FAILED, attachment '{}' is not compressed",
-								 Ref.BucketSegment,
-								 Ref.HashKey,
-								 ToString(HttpContentType::kCbPackage),
-								 Hash);
-						InvalidCount++;
+						Count.New++;
 					}
+					Count.Valid++;
 				}
-				else if (m_CidStore.ContainsChunk(Hash))
+				else
 				{
-					ValidAttachments.emplace_back(Hash);
+					ZEN_WARN("PUT - '{}/{}' '{}' FAILED, attachment '{}' is not compressed",
+							 Ref.BucketSegment,
+							 Ref.HashKey,
+							 ToString(HttpContentType::kCbPackage),
+							 Hash);
+					Count.Invalid++;
 				}
-				TotalCount++;
-			});
+			}
+			else if (m_CidStore.ContainsChunk(Hash))
+			{
+				ValidAttachments.emplace_back(Hash);
+				Count.Valid++;
+			}
+			Count.Total++;
+		});
 
-		if (InvalidCount > 0)
+		if (Count.Invalid > 0)
 		{
 			return Request.WriteResponse(HttpResponseCode::BadRequest, HttpContentType::kText, "Invalid attachment(s)"sv);
 		}
 
-		const bool IsPartialCacheRecord = TotalCount != static_cast<int32_t>(ValidAttachments.size());
-
-		ZEN_DEBUG("PUT - '{}/{}' {} '{}', {}/{} new attachments {}",
+		ZEN_DEBUG("PUT - '{}/{}' {} '{}', attachments '{}/{}/{}' (New/Valid/Total)",
 				  Ref.BucketSegment,
 				  Ref.HashKey,
 				  NiceBytes(Body.GetSize()),
 				  ToString(ContentType),
-				  NewCount,
-				  TotalCount,
-				  IsPartialCacheRecord ? "(PARTIAL)"sv : ""sv);
+				  Count.New,
+				  Count.Valid,
+				  Count.Total);
 
 		IoBuffer CacheRecordValue = CacheRecord.GetBuffer().AsIoBuffer();
 		CacheRecordValue.SetContentType(ZenContentType::kCbObject);
 
 		m_CacheStore.Put(Ref.BucketSegment, Ref.HashKey, {.Value = CacheRecord.GetBuffer().AsIoBuffer()});
 
-		if (StoreUpstream && !IsPartialCacheRecord)
+		const bool IsPartialRecord = Count.Valid != Count.Total;
+
+		if (StoreUpstream && !IsPartialRecord)
 		{
 			ZEN_ASSERT(m_UpstreamCache);
 			auto Result = m_UpstreamCache->EnqueueUpstream({.Type		= ZenContentType::kCbPackage,
@@ -885,30 +892,57 @@ HttpStructuredCacheService::HandleRpcGetCacheRecords(zen::HttpServerRequest& Req
 			[this, &CacheKeys, &CacheValues, &RpcResponse, SkipAttachments](CacheRecordGetCompleteParams&& Params) {
 				if (Params.Record)
 				{
-					Params.Record.IterateAttachments([&](CbFieldView AttachmentHash) {
-						if (const CbAttachment* Attachment = Params.Package.FindAttachment(AttachmentHash.AsHash()))
+					AttachmentCount Count;
+					Params.Record.IterateAttachments([this, &RpcResponse, SkipAttachments, &Params, &Count](CbFieldView HashView) {
+						if (const CbAttachment* Attachment = Params.Package.FindAttachment(HashView.AsHash()))
 						{
 							if (CompressedBuffer Compressed = Attachment->AsCompressedBinary())
 							{
-								m_CidStore.AddChunk(Compressed);
+								auto InsertResult = m_CidStore.AddChunk(Compressed);
+								if (InsertResult.New)
+								{
+									Count.New++;
+								}
+								Count.Valid++;
 
 								if (!SkipAttachments)
 								{
 									RpcResponse.AddAttachment(CbAttachment(Compressed));
 								}
 							}
+							else
+							{
+								ZEN_DEBUG("Uncompressed payload '{}' from upstream cache record '{}/{}'",
+										  HashView.AsHash(),
+										  Params.CacheKey.Bucket,
+										  Params.CacheKey.Hash);
+								Count.Invalid++;
+							}
 						}
+						else if (m_CidStore.ContainsChunk(HashView.AsHash()))
+						{
+							Count.Valid++;
+						}
+						Count.Total++;
 					});
 
-					ZEN_DEBUG("HIT - '{}/{}' {} '{}' (UPSTREAM)",
+					ZEN_DEBUG("HIT - '{}/{}' {} '{}' attachments '{}/{}/{}' (New/Valid/Total) (UPSTREAM)",
 							  Params.CacheKey.Bucket,
 							  Params.CacheKey.Hash,
 							  NiceBytes(Params.Record.GetView().GetSize()),
-							  ToString(HttpContentType::kCbObject));
+							  ToString(HttpContentType::kCbPackage),
+							  Count.New,
+							  Count.Valid,
+							  Count.Total);
 
 					ZEN_ASSERT(Params.KeyIndex < CacheValues.size());
 
-					CacheValues[Params.KeyIndex] = CbObject::Clone(Params.Record).GetBuffer().AsIoBuffer();
+					IoBuffer CacheValue = CbObject::Clone(Params.Record).GetBuffer().AsIoBuffer();
+					CacheValue.SetContentType(ZenContentType::kCbObject);
+
+					CacheValues[Params.KeyIndex] = CacheValue;
+					m_CacheStore.Put(Params.CacheKey.Bucket, Params.CacheKey.Hash, {.Value = CacheValue});
+
 					m_CacheStats.HitCount++;
 					m_CacheStats.UpstreamHitCount++;
 				}