adds tests, fixes a number of problems and adds zap logger for logging

This commit is contained in:
Marcin Czenko 2025-10-23 03:28:02 +02:00
parent 8a16db8f30
commit a3e2ad0341
No known key found for this signature in database
GPG Key ID: A0449219BDBA98AE
5 changed files with 388 additions and 43 deletions

View File

@ -6,12 +6,13 @@ package communities
import (
"context"
"fmt"
"log"
"slices"
"sync"
"time"
"go-codex-client/protobuf"
"go.uber.org/zap"
)
// CodexArchiveProcessor handles processing of downloaded archive data
@ -29,11 +30,11 @@ type CodexArchiveDownloader struct {
communityID string
existingArchiveIDs []string
cancelChan chan struct{} // for cancellation support
logger *zap.Logger
// Progress tracking
totalArchivesCount int
totalDownloadedArchivesCount int
currentArchiveHash string
archiveDownloadProgress map[string]int64 // hash -> bytes downloaded
archiveDownloadCancel map[string]chan struct{}
mu sync.RWMutex
@ -42,6 +43,7 @@ type CodexArchiveDownloader struct {
downloadComplete bool
cancelled bool
pollingInterval time.Duration // configurable polling interval for HasCid checks
pollingTimeout time.Duration // configurable timeout for HasCid polling
// Callbacks
onArchiveDownloaded func(hash string, from, to uint64)
@ -49,18 +51,20 @@ type CodexArchiveDownloader struct {
}
// NewCodexArchiveDownloader creates a new archive downloader
func NewCodexArchiveDownloader(codexClient CodexClientInterface, index *protobuf.CodexWakuMessageArchiveIndex, communityID string, existingArchiveIDs []string, cancelChan chan struct{}) *CodexArchiveDownloader {
func NewCodexArchiveDownloader(codexClient CodexClientInterface, index *protobuf.CodexWakuMessageArchiveIndex, communityID string, existingArchiveIDs []string, cancelChan chan struct{}, logger *zap.Logger) *CodexArchiveDownloader {
return &CodexArchiveDownloader{
codexClient: codexClient,
index: index,
communityID: communityID,
existingArchiveIDs: existingArchiveIDs,
cancelChan: cancelChan,
logger: logger,
totalArchivesCount: len(index.Archives),
totalDownloadedArchivesCount: len(existingArchiveIDs),
archiveDownloadProgress: make(map[string]int64),
archiveDownloadCancel: make(map[string]chan struct{}),
pollingInterval: 1 * time.Second, // Default production polling interval
pollingInterval: 1 * time.Second, // Default production polling interval
pollingTimeout: 30 * time.Second, // Default production polling timeout
}
}
@ -71,6 +75,13 @@ func (d *CodexArchiveDownloader) SetPollingInterval(interval time.Duration) {
d.pollingInterval = interval
}
// SetPollingTimeout sets the timeout for HasCid polling (useful for testing)
func (d *CodexArchiveDownloader) SetPollingTimeout(timeout time.Duration) {
d.mu.Lock()
defer d.mu.Unlock()
d.pollingTimeout = timeout
}
// SetOnArchiveDownloaded sets a callback function to be called when an archive is successfully downloaded
func (d *CodexArchiveDownloader) SetOnArchiveDownloaded(callback func(hash string, from, to uint64)) {
d.onArchiveDownloaded = callback
@ -101,13 +112,6 @@ func (d *CodexArchiveDownloader) GetPendingArchivesCount() int {
return len(d.archiveDownloadCancel)
}
// GetCurrentArchiveHash returns the hash of the currently downloading archive
func (d *CodexArchiveDownloader) GetCurrentArchiveHash() string {
d.mu.RLock()
defer d.mu.RUnlock()
return d.currentArchiveHash
}
// GetArchiveDownloadProgress returns the download progress for a specific archive
func (d *CodexArchiveDownloader) GetArchiveDownloadProgress(hash string) int64 {
d.mu.RLock()
@ -210,7 +214,6 @@ func (d *CodexArchiveDownloader) downloadAllArchives() {
archiveCancelChan := make(chan struct{})
d.mu.Lock()
d.currentArchiveHash = archive.hash
d.archiveDownloadProgress[archive.hash] = 0
d.archiveDownloadCancel[archive.hash] = archiveCancelChan
d.mu.Unlock()
@ -222,53 +225,67 @@ func (d *CodexArchiveDownloader) downloadAllArchives() {
// Trigger archive download and track progress in a goroutine
go func(archiveHash, archiveCid string, archiveFrom, archiveTo uint64, archiveCancel chan struct{}) {
defer func() {
// Always clean up: remove from active downloads and check completion
d.mu.Lock()
delete(d.archiveDownloadCancel, archiveHash)
d.downloadComplete = len(d.archiveDownloadCancel) == 0
d.mu.Unlock()
}()
err := d.triggerSingleArchiveDownload(archiveHash, archiveCid, archiveCancel)
// Update shared state with minimal lock scope
d.mu.Lock()
if err == nil {
d.totalDownloadedArchivesCount++
if err != nil {
// Don't proceed to polling if trigger failed (could be cancellation or other error)
d.logger.Debug("failed to trigger download",
zap.String("cid", archiveCid),
zap.String("hash", archiveHash),
zap.Error(err))
return
}
d.mu.Unlock()
// poll at configured interval until we confirm it's downloaded
// or timeout after 30 seconds
timeout := time.After(30 * time.Second)
// Poll at configured interval until we confirm it's downloaded
// or timeout, or get cancelled
timeout := time.After(d.pollingTimeout)
ticker := time.NewTicker(d.pollingInterval)
defer ticker.Stop()
PollLoop:
for {
select {
case <-timeout:
log.Printf("timeout waiting for CID %s to be available locally", archiveCid)
break PollLoop
d.logger.Debug("timeout waiting for CID to be available locally",
zap.String("cid", archiveCid),
zap.String("hash", archiveHash),
zap.Duration("timeout", d.pollingTimeout))
return // Exit without success callback or count increment
case <-archiveCancel:
d.logger.Debug("download cancelled",
zap.String("cid", archiveCid),
zap.String("hash", archiveHash))
return // Exit without success callback or count increment
case <-ticker.C:
hasCid, err := d.codexClient.HasCid(archiveCid)
if err != nil {
// Log error but continue polling
log.Printf("error checking CID %s: %v", archiveCid, err)
d.logger.Debug("error checking CID availability",
zap.String("cid", archiveCid),
zap.String("hash", archiveHash),
zap.Error(err))
continue
}
if hasCid {
// CID is now available locally
break PollLoop
// CID is now available locally - handle success immediately
d.mu.Lock()
d.totalDownloadedArchivesCount++
d.mu.Unlock()
// Call success callback
if d.onArchiveDownloaded != nil {
d.onArchiveDownloaded(archiveHash, archiveFrom, archiveTo)
}
return // Exit after successful completion
}
}
}
// Update shared state with minimal lock scope
d.mu.Lock()
// Remove from active downloads
delete(d.archiveDownloadCancel, archiveHash)
// Check if all downloads are complete
d.downloadComplete = len(d.archiveDownloadCancel) == 0
d.mu.Unlock()
// Call the callback outside the lock to avoid blocking other operations
if d.onArchiveDownloaded != nil {
d.onArchiveDownloaded(archiveHash, archiveFrom, archiveTo)
}
}(archive.hash, archive.cid, archive.from, archive.to, archiveCancelChan)
}
}

View File

@ -16,6 +16,7 @@ import (
"go-codex-client/protobuf"
"go.uber.org/mock/gomock"
"go.uber.org/zap"
)
// CodexArchiveDownloaderTestifySuite demonstrates testify's suite functionality
@ -67,7 +68,8 @@ func (suite *CodexArchiveDownloaderTestifySuite) TestBasicSingleArchive() {
)
// Create downloader with mock client
downloader := communities.NewCodexArchiveDownloader(suite.mockClient, suite.index, communityID, existingArchiveIDs, cancelChan)
logger := zap.NewNop() // No-op logger for tests
downloader := communities.NewCodexArchiveDownloader(suite.mockClient, suite.index, communityID, existingArchiveIDs, cancelChan, logger)
// Set fast polling interval for tests (10ms instead of default 1s)
downloader.SetPollingInterval(10 * time.Millisecond)
@ -153,7 +155,8 @@ func (suite *CodexArchiveDownloaderTestifySuite) TestMultipleArchives() {
}
// Create downloader
downloader := communities.NewCodexArchiveDownloader(suite.mockClient, index, communityID, existingArchiveIDs, cancelChan)
logger := zap.NewNop() // No-op logger for tests
downloader := communities.NewCodexArchiveDownloader(suite.mockClient, index, communityID, existingArchiveIDs, cancelChan, logger)
downloader.SetPollingInterval(10 * time.Millisecond)
// Track the order in which archives are started (deterministic)
@ -201,6 +204,268 @@ func (suite *CodexArchiveDownloaderTestifySuite) TestMultipleArchives() {
suite.T().Logf(" - Start order (sorted): %v", startOrder)
}
func (suite *CodexArchiveDownloaderTestifySuite) TestCancellationDuringTriggerDownload() {
// Test that cancellation during TriggerDownloadWithContext is handled properly
communityID := "test-community"
existingArchiveIDs := []string{} // No existing archives
cancelChan := make(chan struct{})
// Mock TriggerDownloadWithContext to simulate a cancellation error
suite.mockClient.EXPECT().
TriggerDownloadWithContext(gomock.Any(), "test-cid-1").
Return(nil, assert.AnError). // Return a generic error to simulate failure
Times(1)
// No HasCid calls should be made since TriggerDownload fails
// (this is the key test - we shouldn't proceed to polling)
// Create downloader with mock client
logger := zap.NewNop() // No-op logger for tests
downloader := communities.NewCodexArchiveDownloader(suite.mockClient, suite.index, communityID, existingArchiveIDs, cancelChan, logger)
downloader.SetPollingInterval(10 * time.Millisecond)
// Track callbacks - onArchiveDownloaded should NOT be called on failure
var callbackInvoked bool
var startCallbackInvoked bool
downloader.SetOnArchiveDownloaded(func(hash string, from, to uint64) {
callbackInvoked = true
})
downloader.SetOnStartingArchiveDownload(func(hash string, from, to uint64) {
startCallbackInvoked = true
})
// Start the download
downloader.StartDownload()
// Wait a bit to ensure the goroutine has time to complete
time.Sleep(200 * time.Millisecond)
// Verify the state - download should be marked complete (no pending downloads)
// but no archives should be successfully downloaded
assert.True(suite.T(), startCallbackInvoked, "Start callback should be invoked")
assert.False(suite.T(), callbackInvoked, "Success callback should NOT be invoked on failure")
assert.Equal(suite.T(), 0, downloader.GetTotalDownloadedArchivesCount(), "No archives should be downloaded on failure")
assert.True(suite.T(), downloader.IsDownloadComplete(), "Download should be complete (no pending downloads)")
assert.Equal(suite.T(), 0, downloader.GetPendingArchivesCount(), "No archives should be pending")
suite.T().Log("✅ Cancellation during trigger download test passed")
suite.T().Log(" - TriggerDownload failed as expected")
suite.T().Log(" - No polling occurred (as intended)")
suite.T().Log(" - Success callback was NOT invoked")
}
func (suite *CodexArchiveDownloaderTestifySuite) TestCancellationDuringPolling() {
// Test that cancellation during the polling phase is handled properly
communityID := "test-community"
existingArchiveIDs := []string{} // No existing archives
cancelChan := make(chan struct{})
// Mock successful TriggerDownload
suite.mockClient.EXPECT().
TriggerDownloadWithContext(gomock.Any(), "test-cid-1").
Return(&communities.CodexManifest{CID: "test-cid-1"}, nil).
Times(1)
// Mock polling - allow multiple calls, but we'll cancel before completion
suite.mockClient.EXPECT().
HasCid("test-cid-1").
Return(false, nil).
AnyTimes() // Allow multiple calls since timing is unpredictable
// Create downloader with mock client
logger := zap.NewNop() // No-op logger for tests
downloader := communities.NewCodexArchiveDownloader(suite.mockClient, suite.index, communityID, existingArchiveIDs, cancelChan, logger)
downloader.SetPollingInterval(50 * time.Millisecond) // Longer interval to allow cancellation
downloader.SetPollingTimeout(1 * time.Second) // Short timeout for test (instead of 30s)
// Track callbacks
var successCallbackInvoked bool
var startCallbackInvoked bool
downloader.SetOnArchiveDownloaded(func(hash string, from, to uint64) {
successCallbackInvoked = true
})
downloader.SetOnStartingArchiveDownload(func(hash string, from, to uint64) {
startCallbackInvoked = true
})
// Start the download
downloader.StartDownload()
// Wait for the download to start and first poll to occur
time.Sleep(100 * time.Millisecond)
// Verify initial state
assert.True(suite.T(), startCallbackInvoked, "Start callback should be invoked")
assert.Equal(suite.T(), 1, downloader.GetPendingArchivesCount(), "Should have 1 pending download")
assert.False(suite.T(), downloader.IsDownloadComplete(), "Download should not be complete yet")
// Cancel the entire operation
close(cancelChan)
// Wait for cancellation to propagate
require.Eventually(suite.T(), func() bool {
return downloader.IsDownloadComplete()
}, 2*time.Second, 50*time.Millisecond, "Download should complete after cancellation")
// Verify final state
assert.False(suite.T(), successCallbackInvoked, "Success callback should NOT be invoked on cancellation")
assert.Equal(suite.T(), 0, downloader.GetPendingArchivesCount(), "No archives should be pending after cancellation")
assert.True(suite.T(), downloader.IsCancelled(), "Downloader should be marked as cancelled")
suite.T().Log("✅ Cancellation during polling test passed")
suite.T().Log(" - TriggerDownload succeeded")
suite.T().Log(" - Polling started but was cancelled")
suite.T().Log(" - Success callback was NOT invoked")
suite.T().Log(" - Download marked as cancelled")
}
func (suite *CodexArchiveDownloaderTestifySuite) TestPollingTimeout() {
// Test that polling timeout is handled properly (no success callback)
communityID := "test-community"
existingArchiveIDs := []string{} // No existing archives
cancelChan := make(chan struct{})
// Mock successful TriggerDownload
suite.mockClient.EXPECT().
TriggerDownloadWithContext(gomock.Any(), "test-cid-1").
Return(&communities.CodexManifest{CID: "test-cid-1"}, nil).
Times(1)
// Mock polling to always return false (simulating timeout)
suite.mockClient.EXPECT().
HasCid("test-cid-1").
Return(false, nil).
AnyTimes() // Will be called multiple times until timeout
// Create downloader with mock client
logger := zap.NewNop() // No-op logger for tests
downloader := communities.NewCodexArchiveDownloader(suite.mockClient, suite.index, communityID, existingArchiveIDs, cancelChan, logger)
downloader.SetPollingInterval(10 * time.Millisecond) // Fast polling for test
downloader.SetPollingTimeout(100 * time.Millisecond) // Short timeout for test (instead of 30s)
// Track callbacks
var successCallbackInvoked bool
var startCallbackInvoked bool
downloader.SetOnArchiveDownloaded(func(hash string, from, to uint64) {
successCallbackInvoked = true
})
downloader.SetOnStartingArchiveDownload(func(hash string, from, to uint64) {
startCallbackInvoked = true
})
// Start the download
downloader.StartDownload()
// Wait for timeout (100ms configured timeout)
// We'll wait a bit longer to ensure timeout occurs
require.Eventually(suite.T(), func() bool {
return downloader.IsDownloadComplete()
}, 500*time.Millisecond, 50*time.Millisecond, "Download should complete after timeout")
// Verify state after timeout
assert.True(suite.T(), startCallbackInvoked, "Start callback should be invoked")
assert.False(suite.T(), successCallbackInvoked, "Success callback should NOT be invoked on timeout")
assert.Equal(suite.T(), 0, downloader.GetTotalDownloadedArchivesCount(), "No archives should be downloaded on timeout")
assert.Equal(suite.T(), 0, downloader.GetPendingArchivesCount(), "No archives should be pending after timeout")
assert.True(suite.T(), downloader.IsDownloadComplete(), "Download should be complete")
suite.T().Log("✅ Polling timeout test passed")
suite.T().Log(" - TriggerDownload succeeded")
suite.T().Log(" - Polling timed out after 100ms (fast test)")
suite.T().Log(" - Success callback was NOT invoked")
}
func (suite *CodexArchiveDownloaderTestifySuite) TestWithExistingArchives() {
// Test with some archives already downloaded (existing archive IDs)
index := &protobuf.CodexWakuMessageArchiveIndex{
Archives: map[string]*protobuf.CodexWakuMessageArchiveIndexMetadata{
"archive-1": {
Cid: "cid-1",
Metadata: &protobuf.WakuMessageArchiveMetadata{From: 1000, To: 2000},
},
"archive-2": {
Cid: "cid-2",
Metadata: &protobuf.WakuMessageArchiveMetadata{From: 2000, To: 3000},
},
"archive-3": {
Cid: "cid-3",
Metadata: &protobuf.WakuMessageArchiveMetadata{From: 3000, To: 4000},
},
},
}
communityID := "test-community"
// Simulate that we already have archive-1 and archive-3
existingArchiveIDs := []string{"archive-1", "archive-3"}
cancelChan := make(chan struct{})
// Only archive-2 should be downloaded (not in existingArchiveIDs)
suite.mockClient.EXPECT().
TriggerDownloadWithContext(gomock.Any(), "cid-2").
Return(&communities.CodexManifest{CID: "cid-2"}, nil).
Times(1) // Only one call expected
// Only archive-2 should be polled
gomock.InOrder(
suite.mockClient.EXPECT().HasCid("cid-2").Return(false, nil),
suite.mockClient.EXPECT().HasCid("cid-2").Return(true, nil),
)
// Create downloader with existing archives
logger := zap.NewNop() // No-op logger for tests
downloader := communities.NewCodexArchiveDownloader(suite.mockClient, index, communityID, existingArchiveIDs, cancelChan, logger)
downloader.SetPollingInterval(10 * time.Millisecond)
// Track which archives are started and completed
var startedArchives []string
var completedArchives []string
downloader.SetOnStartingArchiveDownload(func(hash string, from, to uint64) {
startedArchives = append(startedArchives, hash)
})
downloader.SetOnArchiveDownloaded(func(hash string, from, to uint64) {
completedArchives = append(completedArchives, hash)
})
// Verify initial state - should start with 2 existing archives counted
assert.Equal(suite.T(), 3, downloader.GetTotalArchivesCount(), "Should have 3 total archives")
assert.Equal(suite.T(), 2, downloader.GetTotalDownloadedArchivesCount(), "Should start with 2 existing archives")
assert.False(suite.T(), downloader.IsDownloadComplete(), "Should not be complete initially")
// Start download
downloader.StartDownload()
// Wait for download to complete
require.Eventually(suite.T(), func() bool {
return downloader.IsDownloadComplete()
}, 5*time.Second, 100*time.Millisecond, "Download should complete within 5 seconds")
// Verify final state
assert.True(suite.T(), downloader.IsDownloadComplete(), "Download should be complete")
assert.Equal(suite.T(), 3, downloader.GetTotalDownloadedArchivesCount(), "Should have 3 total downloaded (2 existing + 1 new)")
// Verify only missing archive was processed
assert.Len(suite.T(), startedArchives, 1, "Should have started exactly 1 archive download")
assert.Contains(suite.T(), startedArchives, "archive-2", "Should have started archive-2")
assert.NotContains(suite.T(), startedArchives, "archive-1", "Should NOT have started archive-1 (existing)")
assert.NotContains(suite.T(), startedArchives, "archive-3", "Should NOT have started archive-3 (existing)")
assert.Len(suite.T(), completedArchives, 1, "Should have completed exactly 1 archive download")
assert.Contains(suite.T(), completedArchives, "archive-2", "Should have completed archive-2")
suite.T().Log("✅ Existing archives test passed")
suite.T().Logf(" - Started with %d existing archives", len(existingArchiveIDs))
suite.T().Logf(" - Downloaded %d missing archives", len(completedArchives))
suite.T().Logf(" - Final count: %d total", downloader.GetTotalDownloadedArchivesCount())
}
// Run the test suite
func TestCodexArchiveDownloaderSuite(t *testing.T) {
suite.Run(t, new(CodexArchiveDownloaderTestifySuite))

View File

@ -0,0 +1,55 @@
package main
import (
"go-codex-client/communities"
"go-codex-client/protobuf"
"go.uber.org/zap"
)
func main() {
// Create a production logger with JSON output
logger, _ := zap.NewProduction()
defer logger.Sync()
// Or create a development logger with console output
// logger, _ := zap.NewDevelopment()
// Example usage in production
var (
codexClient communities.CodexClientInterface = nil // your actual client
index *protobuf.CodexWakuMessageArchiveIndex = nil // your index
communityID = "your-community-id"
existingArchives = []string{"existing-hash-1", "existing-hash-2"}
cancelChan = make(chan struct{})
)
// Create downloader with structured logging
downloader := communities.NewCodexArchiveDownloader(
codexClient,
index,
communityID,
existingArchives,
cancelChan,
logger,
)
// Configure for production use
downloader.SetOnArchiveDownloaded(func(hash string, from, to uint64) {
logger.Info("archive download completed",
zap.String("hash", hash),
zap.String("community", communityID),
zap.Uint64("from", from),
zap.Uint64("to", to))
})
downloader.SetOnStartingArchiveDownload(func(hash string, from, to uint64) {
logger.Info("starting archive download",
zap.String("hash", hash),
zap.String("community", communityID),
zap.Uint64("from", from),
zap.Uint64("to", to))
})
// Start downloads - all internal logging will now use structured zap logging
// downloader.StartDownload()
}

2
go.mod
View File

@ -5,11 +5,13 @@ go 1.23.0
require (
github.com/stretchr/testify v1.11.1
go.uber.org/mock v0.6.0
go.uber.org/zap v1.27.0
google.golang.org/protobuf v1.34.1
)
require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
go.uber.org/multierr v1.10.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

6
go.sum
View File

@ -6,8 +6,14 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
go.uber.org/mock v0.6.0 h1:hyF9dfmbgIX5EfOdasqLsWD6xqpNZlXblLB/Dbnwv3Y=
go.uber.org/mock v0.6.0/go.mod h1:KiVJ4BqZJaMj4svdfmHM0AUx4NJYO8ZNpPnZn1Z+BBU=
go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ=
go.uber.org/multierr v1.10.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8=
go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg=