CBG-4280 consolidate Attachment on document

[torcolvin]

CBG-4280 consolidate Attachment on document

• merge Document.SyncData.Attachments, Document.GlobalSyncData.GlobalAttachments, and Document.DocAttachments into Document._globalSync.Attachments
• Document.SyncData.Attachments becomes Document.SyncData.AttachmentsPre4dot0 and is used for unmarshalling, but should be cleared before writing a document
• rename GlobalSyncData.GlobalAttachments to GlobalSyncData.Attachments

I feel pretty comfortable with the amount of testing that this has, especially after I did 7ded498

I've considered whether to make GlobalSyncData not embedded into Document, but ultimately I decided not to for the ease of referring to Document.Attachments in the code. This makes the in memory data structure s

Things I don't love:

• Document.SyncData.AttachmentsPre4dot0 has to exist to unmarshal SyncData, but we need to be sure to clear this when we are writing this data. This is handled in unmarshalDocumentWithXattrs. An alternate approach to what I did would be to create a MarshalJSON function to make sure that this field never got marshalled, but I couldn't think of a good way to do this.
• Document.MarshalJSON has to re-add attachments into SyncData into order to have ImportDoc work correctly. This is already noted as weird in this TODO

  sync_gateway/db/import.go

  Line 78 in feebe5a

  // TODO: We need to remarshal the existing doc into bytes. Less performance overhead than the previous bucket op to get the value in WriteUpdateWithXattr,

  . I can't think of a place that this is called outside ImportDoc because you almost never want to marshal metadata into an inline body. I think this hack in Document.MarshalJSON could be removed if ImportDoc got refactored. See TestLegacyAttachmentMigrationToGlobal for the only code that tests this pathway.

Pre-review checklist

• Removed debug logging (fmt.Print, log.Print, ...)
• Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
• Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Integration Tests

• GSI=true,xattrs=true https://jenkins.sgwdev.com/job/SyncGateway-Integration/3227/

[Copilot]

Pull Request Overview

This pull request consolidates attachment metadata storage in Sync Gateway documents by merging three separate attachment storage locations into a single unified location.

• Merges Document.SyncData.Attachments, Document.GlobalSyncData.GlobalAttachments, and Document.DocAttachments into Document._globalSync.Attachments
• Introduces Document.SyncData.AttachmentsPre4dot0 for backward compatibility during unmarshalling
• Provides getter/setter methods (Attachments() and SetAttachments()) for cleaner access to attachment metadata

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
db/document.go	Core attachment consolidation logic with new unified storage and access methods
db/crud.go	Updates document operations to use new attachment accessor methods
db/blip_handler.go	Updates BLIP protocol handling to use new attachment methods
db/import.go	Updates import functionality to use new attachment methods
db/attachment.go	Updates attachment storage to use new accessor methods
Multiple test files	Updates test assertions to use new attachment accessor methods

Comments suppressed due to low confidence (1)

rest/attachment_test.go:2837

• The test configuration change from nil to &RestTesterConfig{AutoImport: base.Ptr(false)} modifies test behavior significantly. Ensure this change is intentional and doesn't affect the test's validity.

	rt := NewRestTester(t, &RestTesterConfig{AutoImport: base.Ptr(false)})

[gregns1]

LGTM, just couple of nit pick comments