RavenDB-16815 - do todos

src/Raven.Client/Documents/Attachments/RetiredAttachmentsConfiguration.cs

@@ -16,6 +16,7 @@ public sealed class RetiredAttachmentsConfiguration : IDynamicJson
         public long? RetireFrequencyInSec { get; set; }
         public long? MaxItemsToProcess { get; set; }
 
+        // TODO: egor we need to make those setting per collection
         /// <summary>
         /// Purge the retired attachments when the document is deleted.
         /// Default: false

src/Raven.Client/Documents/Operations/Attachments/Retired/DeleteRetiredAttachmentsOperation.cs

@@ -12,15 +12,16 @@ namespace Raven.Client.Documents.Operations.Attachments.Retired
     public sealed class DeleteRetiredAttachmentsOperation : IOperation
     {
         private readonly IEnumerable<AttachmentRequest> _attachments;
-
-        public DeleteRetiredAttachmentsOperation(IEnumerable<AttachmentRequest> attachments)
+        private readonly bool _storageOnly;
+        public DeleteRetiredAttachmentsOperation(IEnumerable<AttachmentRequest> attachments, bool storageOnly = false)
         {
             _attachments = attachments;
+            _storageOnly = storageOnly;
         }
 
         public RavenCommand GetCommand(IDocumentStore store, DocumentConventions conventions, JsonOperationContext context, HttpCache cache)
         {
-            return new DeleteRetiredAttachmentsCommand(conventions, context, _attachments);
+            return new DeleteRetiredAttachmentsCommand(conventions, context, _attachments, _storageOnly);
         }
 
         internal sealed class DeleteRetiredAttachmentsCommand : RavenCommand
@@ -29,18 +30,26 @@ internal sealed class DeleteRetiredAttachmentsCommand : RavenCommand
             private readonly JsonOperationContext _context;
             internal IEnumerable<AttachmentRequest> Attachments { get; }
             internal List<AttachmentDetails> AttachmentsMetadata { get; } = new List<AttachmentDetails>();
+            private readonly bool _storageOnly;
 
-            public DeleteRetiredAttachmentsCommand(DocumentConventions conventions, JsonOperationContext context, IEnumerable<AttachmentRequest> attachments)
+            public DeleteRetiredAttachmentsCommand(DocumentConventions conventions, JsonOperationContext context, IEnumerable<AttachmentRequest> attachments, bool storageOnly)
             {
                 _conventions = conventions ?? throw new ArgumentNullException(nameof(conventions));
                 _context = context;
                 Attachments = attachments;
                 ResponseType = RavenCommandResponseType.Empty;
+                _storageOnly = storageOnly;
             }
 
             public string GetUrl(ServerNode node)
             {
-                return $"{node.Url}/databases/{node.Database}/attachments/retire/bulk";
+                var url = $"{node.Url}/databases/{node.Database}/attachments/retire/bulk";
+                if (_storageOnly)
+                {
+                    url += "&storageOnly=true";
+                }
+
+                return url;
             }
 
             public override HttpRequestMessage CreateRequest(JsonOperationContext ctx, ServerNode node, out string url)

src/Raven.Server/Documents/AttachmentsStorage.cs

@@ -389,7 +389,6 @@ void SetTableValue(TableValueBuilder tvb, Slice cv)
                                                     {
                                                         // we cannot delete from cloud since PurgeOnDelete is false
                                                         DeleteInternal(context, existingKey, existingEtag, existingHash, changeVector, lastModifiedTicks, flags: DocumentFlags.None, existingAttachmentFlags, existingRetireAtTicks, collectionName.Name, storageOnly: true);
-
                                                     }
                                                     else
                                                     {
@@ -398,7 +397,7 @@ void SetTableValue(TableValueBuilder tvb, Slice cv)
                                                 }
                                                 else
                                                 {
-                                                    DeleteInternal(context, existingKey, existingEtag, existingHash, changeVector, lastModifiedTicks, flags: DocumentFlags.None, existingAttachmentFlags, existingRetireAtTicks, collectionName.Name, storageOnly: false);
+                                                    DeleteInternal(context, existingKey, existingEtag, existingHash, changeVector, lastModifiedTicks, flags: DocumentFlags.None, existingAttachmentFlags, existingRetireAtTicks, collectionName.Name, storageOnly: true);
                                                 }
                                             }
                                             else
@@ -529,13 +528,6 @@ private void TryDeleteRetiredAttachment(DocumentsOperationContext context, Slice
                 }
 
                 RetiredAttachmentsStorage.PutDelete(context, keySlice, DateTime.UtcNow.Ticks, collection);
-
-                // I think it should be user responsibility to make sure he has same config as when he retired the attachment
-                //TODO: egor how can I know where it was uploaded ? 
-                // do I need to store blob url in attachments metadata?
-                // do I even care about it ? 
-                // THe issue is that the configuration can change after I retired the attachment and I will not know where it was uploaded
-                // also if the configuration is disabled, should I fail the operation?
             }
             else
             {
@@ -561,7 +553,6 @@ public void PutDirect(DocumentsOperationContext context, Slice key, Slice name,
             }
             Debug.Assert(changeVector != null);
             DeleteTombstoneIfNeeded(context, key);
-            //TODO: egor check if we have @retire-at in metadata and put it in RetiredAttachmentsStorage
             var table = context.Transaction.InnerTransaction.OpenTable(AttachmentsSchema, AttachmentsMetadataSlice);
             using (Slice.From(context.Allocator, changeVector, out var changeVectorSlice))
             using (table.Allocate(out TableValueBuilder tvb))
@@ -582,10 +573,13 @@ public void PutDirect(DocumentsOperationContext context, Slice key, Slice name,
                 tvb.Add(collection.Content.Ptr, collection.Size);
                 table.Set(tvb);
             }
-            //TODO: egor what if we restore huge dump, all attachments have retireAt and we do putdirect, in will be stored in the storage forever if there is no configuration.
-            //I think there is nothing I can do?
-            if (isRevision == false && retireAt.HasValue)
-                RetiredAttachmentsStorage.Put(context, key, retireAt.Value.GetDefaultRavenFormat());
+
+            if (isRevision == false)
+            {
+                // TODO: egor do I need to check for retired config here ?
+                if (flags.Contain(AttachmentFlags.Retired) == false && retireAt.HasValue)
+                    RetiredAttachmentsStorage.Put(context, key, retireAt.Value.GetDefaultRavenFormat());
+            }
 
             _documentDatabase.Metrics.Attachments.PutsPerSec.MarkSingleThreaded(1);
         }
@@ -1352,7 +1346,7 @@ public MoveAttachmentDetailsServer MoveAttachment(DocumentsOperationContext cont
             var attachment = GetAttachment(context, sourceDocumentId, sourceName, AttachmentType.Document, changeVector, hash, contentType, usePartialKey);
             if (attachment == null)
                 AttachmentDoesNotExistException.ThrowFor(sourceDocumentId, sourceName);
-
+            //TODO: egor can I do this for retired?
             var result = PutAttachment(context, destinationDocumentId, destinationName, attachment.ContentType, attachment.Base64Hash.ToString(), attachment.Flags,attachment.Size, retireAtDt: null, string.Empty, attachment.Stream, extractCollectionName: extractCollectionName);
             DeleteAttachment(context, sourceDocumentId, sourceName, changeVector, out var sourceCollectionName, updateDocument, hash, contentType, usePartialKey, extractCollectionName: extractCollectionName);
 
@@ -1499,12 +1493,12 @@ public void DeleteAttachmentConflicts(DocumentsOperationContext context, Slice l
                 }
 
                 if (attachmentFoundInResolveDocument == false)
-                    DeleteAttachmentDirect(context, lowerId, conflictName, conflictContentType, conflictHash, changeVector, collection.Name);
+                    DeleteAttachmentDirect(context, lowerId, conflictName, conflictContentType, conflictHash, changeVector);
             }
         }
 
         private void DeleteAttachmentDirect(DocumentsOperationContext context, Slice lowerId, LazyStringValue conflictName,
-            LazyStringValue conflictContentType, LazyStringValue conflictHash, string changeVector, string collection)
+            LazyStringValue conflictContentType, LazyStringValue conflictHash, string changeVector)
         {
             using (DocumentIdWorker.GetSliceFromId(context, conflictName, out Slice lowerName))
             using (DocumentIdWorker.GetSliceFromId(context, conflictContentType, out Slice lowerContentType))
@@ -1514,7 +1508,7 @@ private void DeleteAttachmentDirect(DocumentsOperationContext context, Slice low
                 base64Hash, lowerContentType.Content.Ptr, lowerContentType.Size, AttachmentType.Document, Slices.Empty, out Slice keySlice))
             {
                 var lastModifiedTicks = _documentDatabase.Time.GetUtcNow().Ticks;
-                DeleteAttachmentDirect(context, keySlice, false, null, null, changeVector, lastModifiedTicks);
+                DeleteAttachmentDirect(context, keySlice, false, null, null, changeVector, lastModifiedTicks, storageOnly: true);
             }
         }
 
@@ -1674,7 +1668,6 @@ private void DeleteAttachmentsOfDocumentInternal(DocumentsOperationContext conte
                     {
                         var etag = TableValueToEtag((int)AttachmentsTable.Etag, ref before.Reader);
 
-                        //TODO: egor this will delete the attachments from cloud, do we want that ???? Maybe this should be part of the config ? I mean now we delete doc with retired attachemnt, it will delete the retire attachment from cloud!
                         var attachmentFlags = TableValueToAttachmentFlags((int)AttachmentsTable.Flags, ref before.Reader);
                         var retireAtTicks = TableValueToLong((int)AttachmentsTable.RetireAt, ref before.Reader);
 

src/Raven.Server/Documents/Handlers/Batches/Commands/MergedBatchCommand.cs

@@ -13,6 +13,7 @@
 using Raven.Server.Documents.TimeSeries;
 using Raven.Server.Documents.TransactionMerger.Commands;
 using Raven.Server.ServerWide.Context;
+using Raven.Server.Utils;
 using Sparrow.Json.Parsing;
 
 namespace Raven.Server.Documents.Handlers.Batches.Commands;
@@ -217,7 +218,9 @@ protected override long ExecuteCmd(DocumentsOperationContext context)
                     break;
 
                 case CommandType.AttachmentDELETE:
-                    Database.DocumentsStorage.AttachmentsStorage.DeleteAttachment(context, cmd.Id, cmd.Name, cmd.ChangeVector, out var collectionName, updateDocument: false, extractCollectionName: ModifiedCollections is not null);
+
+                    bool storageOnly = Database.ReadDatabaseRecord().RetiredAttachments is not { Disabled: false, PurgeOnDelete: true };
+                    Database.DocumentsStorage.AttachmentsStorage.DeleteAttachment(context, cmd.Id, cmd.Name, cmd.ChangeVector, out var collectionName, updateDocument: false, extractCollectionName: ModifiedCollections is not null, storageOnly: storageOnly);
 
                     if (collectionName != null)
                         ModifiedCollections?.Add(collectionName.Name);

src/Raven.Server/Documents/Handlers/Processors/Attachments/AttachmentHandlerProcessorForBulkDeleteAttachment.cs

@@ -42,13 +42,19 @@ protected override async ValueTask DeleteAttachmentAsync(DocumentsOperationConte
             if (attachmentRequests.Count == 0)
                 return;
 
+            MergedDeleteAttachmentsCommand cmd = MergedDeleteAttachmentsCommand(attachmentRequests);
+
+            await RequestHandler.Database.TxMerger.Enqueue(cmd);
+        }
+
+        protected virtual MergedDeleteAttachmentsCommand MergedDeleteAttachmentsCommand(List<AttachmentRequest> attachmentRequests)
+        {
             var cmd = new MergedDeleteAttachmentsCommand
             {
                 Database = RequestHandler.Database,
                 Deletes = attachmentRequests
             };
-
-            await RequestHandler.Database.TxMerger.Enqueue(cmd);
+            return cmd;
         }
 
         protected virtual void CheckAttachmentFlagAndThrowIfNeeded(DocumentsOperationContext context, string docId, string name)
@@ -60,12 +66,13 @@ internal sealed class MergedDeleteAttachmentsCommand : MergedTransactionCommand<
     {
         public List<AttachmentRequest> Deletes;
         public DocumentDatabase Database;
+        public bool StorageOnly;
 
         protected override long ExecuteCmd(DocumentsOperationContext context)
         {
             foreach (var delete in Deletes)
             {
-                Database.DocumentsStorage.AttachmentsStorage.DeleteAttachment(context, delete.DocumentId, delete.Name, null, collectionName: out _);
+                Database.DocumentsStorage.AttachmentsStorage.DeleteAttachment(context, delete.DocumentId, delete.Name, null, collectionName: out _, storageOnly: StorageOnly);
             }
 
             return 1;
@@ -74,17 +81,17 @@ protected override long ExecuteCmd(DocumentsOperationContext context)
         public override IReplayableCommandDto<DocumentsOperationContext, DocumentsTransaction,
             MergedTransactionCommand<DocumentsOperationContext, DocumentsTransaction>> ToDto(DocumentsOperationContext context)
         {
-            return new MergedDeleteAttachmentsCommandDto { Deletes = Deletes };
+            return new MergedDeleteAttachmentsCommandDto { Deletes = Deletes, StorageOnly = StorageOnly };
         }
     }
 
     internal sealed class MergedDeleteAttachmentsCommandDto : IReplayableCommandDto<DocumentsOperationContext, DocumentsTransaction, MergedDeleteAttachmentsCommand>
     {
         public List<AttachmentRequest> Deletes;
-
+        public bool StorageOnly;
         public MergedDeleteAttachmentsCommand ToCommand(DocumentsOperationContext context, DocumentDatabase database)
         {
-            return new MergedDeleteAttachmentsCommand { Deletes = Deletes, Database = database };
+            return new MergedDeleteAttachmentsCommand { Deletes = Deletes, Database = database , StorageOnly = StorageOnly };
         }
     }
 }

src/Raven.Server/Documents/Handlers/Processors/Attachments/AttachmentHandlerProcessorForDeleteAttachment.cs

@@ -43,8 +43,6 @@ protected virtual void CheckAttachmentFlagAndThrowIfNeeded(DocumentsOperationCon
 
         public static void CheckAttachmentFlagAndThrowIfNeededInternal(DocumentsOperationContext context, DatabaseRequestHandler requestHandler, string docId, string name)
         {
-            //TODO: egor I have CV, do I need to pass it here? CHeck in future (if test pass when I add it)
-            //TODO: egor shouldn't Attachment be IDisposable? it has lsv & Stream that should be disposed
             //TODO: egor sharding handler?
             var attachment = requestHandler.Database.DocumentsStorage.AttachmentsStorage.GetAttachment(context, docId, name, AttachmentType.Document, changeVector: null);
             if (attachment == null)

src/Raven.Server/Documents/Handlers/Processors/Attachments/Retired/RetiredAttachmentHandlerProcessorForBulkDelete.cs

@@ -1,4 +1,6 @@
-using JetBrains.Annotations;
+using System.Collections.Generic;
+using JetBrains.Annotations;
+using Raven.Client.Documents.Operations.Attachments;
 using Raven.Server.ServerWide.Context;
 
 namespace Raven.Server.Documents.Handlers.Processors.Attachments.Retired
@@ -9,6 +11,15 @@ public RetiredAttachmentHandlerProcessorForBulkDelete([NotNull] DatabaseRequestH
         {
         }
 
+        protected override MergedDeleteAttachmentsCommand MergedDeleteAttachmentsCommand(List<AttachmentRequest> attachmentRequests)
+        {
+            var cmd = base.MergedDeleteAttachmentsCommand(attachmentRequests);
+            var storageOnly = RequestHandler.GetBoolValueQueryString("storageOnly", required: false) ?? false;
+
+            cmd.StorageOnly = storageOnly;
+            return cmd;
+        }
+
         protected override void CheckAttachmentFlagAndThrowIfNeeded(DocumentsOperationContext context, string docId, string name)
         {
             RetiredAttachmentHandlerProcessorForDelete.CheckRetiredAttachmentFlagAndThrowIfNeededInternal(context, RequestHandler, docId, name);

src/Raven.Server/Documents/Handlers/RetiredAttachmentHandler.cs

@@ -41,15 +41,15 @@ public async Task DeleteAttachments()
                 await processor.ExecuteAsync();
         }
 
-        //TODO: egor do I Need a retired version of this?
+        //TODO: egor do I Need a retired version of this? this return same result as regular endpoint
         [RavenAction("/databases/*/debug/attachments/retire/hash", "GET", AuthorizationStatus.ValidUser, EndpointType.Read, DisableOnCpuCreditsExhaustion = true)]
         public async Task GetHashCount()
         {
             using (var processor = new AttachmentHandlerProcessorForGetHashCount(this))
                 await processor.ExecuteAsync();
         }
 
-        //TODO: egor do I Need a retired version of this?
+        //TODO: egor do I Need a retired version of this? this return same result as regular endpoint
         [RavenAction("/databases/*/debug/attachments/retire/metadata", "GET", AuthorizationStatus.ValidUser, EndpointType.Read, DisableOnCpuCreditsExhaustion = true)]
         public async Task GetAttachmentMetadataWithCounts()
         {

src/Raven.Server/Documents/PeriodicBackup/BackupUploader.cs

@@ -235,7 +235,7 @@ private string GetArchiveDescription()
 
             if (backupType.HasValue)
             {
-                description = GetBackupDescription(backupType.Value, _isFullBackup);
+                description = GetBackupDescription();
             }
             else
             {