Skip to content

Comments

[Refactor] Restructure code and add necessary tests#192

Merged
Ghass-M merged 6 commits intofeature/improve-upload-managementfrom
feature/improve-upload-management-measure-metrics
Aug 26, 2025
Merged

[Refactor] Restructure code and add necessary tests#192
Ghass-M merged 6 commits intofeature/improve-upload-managementfrom
feature/improve-upload-management-measure-metrics

Conversation

@Ghass-M
Copy link
Contributor

@Ghass-M Ghass-M commented Aug 25, 2025

Summary

  • Metrics: Refined per-slice upload metrics and captured worker TaskId to observe parallelism.
  • Error handling: Replaced single LastException with aggregated Exceptions list.
  • Tests: Added integration tests validating slicing, parallelism, and total uploaded length; updated unit tests accordingly.
  • Cleanup: Minor code cleanups and API alignment.

Why

  • Bandwidth computation per slice was noisy and unnecessary; external computation can derive rates from bytes and elapsed time.
  • Aggregating errors provides richer diagnostics and avoids losing earlier failures.
  • Integration coverage ensures slicing behavior and concurrency limits are correct across scenarios.

What changed

  • SliceUploadMetric:
    • Added: TaskId.
    • Renamed: DurationMsElapsedtimeMs.
    • Removed: BandwidthKbps.
  • UploadProgressState:
    • Removed: LastException.
    • Use Exceptions list for error aggregation.
  • FileSlicer / FileUploadWorker:
    • Stop setting LastException; append to Exceptions instead.
    • Record TaskId, PartNumber, Bytes, ElapsedtimeMs for each slice.
  • FileUploadProcessor:
    • After upload, throw with the last aggregated exception if any exist.
    • Use null-safe count checks.
  • Tests:
    • New integration suite Upload_SliceAndParallelism_Tests with a stub IUploadStrategy and IFileTransferApiClient to:
      • Validate one-slice uploads, multi-slice uploads, and two-file uploads.
      • Assert total slice counts, distinct worker TaskId counts, and final uploaded length.
    • Updated UploadProgressState unit tests to use Exceptions (including null, different types, inner exceptions, messages).
    • Minor cleanup in FileUploaderSliceTests (remove unused using).

Breaking changes

  • Public model changes:
    • SliceUploadMetric: property rename/removal and new property added.
    • UploadProgressState: LastException removed in favor of Exceptions.
  • Impact expected to be internal to the client; downstream consumers must update references accordingly.

paul-fresquet and others added 6 commits August 23, 2025 07:03
…-upload-management-measure-metrics

# Conflicts:
#	src/ByteSync.Client/Business/Communications/Transfers/SliceUploadMetric.cs
#	src/ByteSync.Client/Business/Communications/Transfers/UploadProgressState.cs
#	src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileSlicer.cs
#	src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileUploadProcessor.cs
#	src/ByteSync.Client/Services/Communications/Transfers/Uploading/FileUploadWorker.cs
#	tests/ByteSync.Client.Tests/Services/Communications/Transfers/Uploading/FileSlicerMetricsTests.cs
#	tests/ByteSync.Client.Tests/Services/Communications/Transfers/Uploading/FileUploadWorkerMetricsTests.cs
@sonarqubecloud
Copy link

@Ghass-M Ghass-M requested a review from paul-fresquet August 25, 2025 14:38
@Ghass-M Ghass-M merged commit c2b4704 into feature/improve-upload-management Aug 26, 2025
39 of 41 checks passed
@Ghass-M Ghass-M deleted the feature/improve-upload-management-measure-metrics branch August 26, 2025 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants