Skip to content

Comments

[feature] Improve upload management adaptive controller#194

Merged
Ghass-M merged 12 commits intofeature/improve-upload-managementfrom
feature/improve-upload-management-adaptive-controller
Aug 27, 2025
Merged

[feature] Improve upload management adaptive controller#194
Ghass-M merged 12 commits intofeature/improve-upload-managementfrom
feature/improve-upload-management-adaptive-controller

Conversation

@Ghass-M
Copy link
Contributor

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

Summary

Wire IAdaptiveUploadController into client-side factories so the adaptive upload controller is constructed and passed down into the upload pipeline.

Changes

  • Add IAdaptiveUploadController resolution and injection in:
    • src/ByteSync.Client/Factories/FileUploadProcessorFactory.cs
    • src/ByteSync.Client/Factories/FileUploaderFactory.cs

These edits ensure the adaptive controller instance is available to IFileUploadProcessor and IFileUploader via DI, enabling adaptive chunk sizing/parallelism at runtime.

Rationale

Prepares the client upload flow to leverage adaptive bandwidth control by providing the controller at composition time. This isolates policy/heuristics from factories and keeps consumers DI-friendly.

Technical Notes

  • Both factories resolve IAdaptiveUploadController from the existing DI context and pass it as a typed parameter.
  • No public API changes in factories; the additional constructor parameter is resolved internally.

Backward Compatibility

  • Safe: DI resolution only. No behavior change unless the downstream constructors require the new dependency (they do in the corresponding feature branch).
  • No config/migration required.

Risks

  • Misconfigured DI could surface as runtime resolution errors if IAdaptiveUploadController is not registered.
  • Ensure IAdaptiveUploadController and its implementation are registered in the client module(s).

Testing

Manual smoke test suggestions:

  • Launch a client build that includes the adaptive upload feature.
  • Start an upload and confirm no DI resolution errors occur.
  • Validate logs to ensure the controller is instantiated once per upload pipeline and methods are invoked during upload.

Checklist

  • Inject controller in FileUploadProcessorFactory
  • Inject controller in FileUploaderFactory
  • No public API changes in factory interfaces
  • Build and tests pass locally (please run commands above)

@paul-fresquet paul-fresquet requested a review from Copilot August 27, 2025 05:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR integrates the adaptive upload controller into the client-side upload pipeline by injecting IAdaptiveUploadController into the file upload factories. This enables dynamic bandwidth control through adaptive chunk sizing and parallelism during file uploads.

  • Modify FileUploadProcessorFactory and FileUploaderFactory to resolve and inject IAdaptiveUploadController
  • Update upload components to use adaptive methods instead of fixed parameters
  • Add comprehensive test coverage for the new adaptive upload functionality

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/ByteSync.Client/Factories/FileUploadProcessorFactory.cs Injects IAdaptiveUploadController into the file upload processor
src/ByteSync.Client/Factories/FileUploaderFactory.cs Injects IAdaptiveUploadController into the file uploader
src/ByteSync.Client/Services/Communications/Transfers/Uploading/ Implements adaptive upload logic and adds adaptive methods
src/ByteSync.Client/Interfaces/Controls/Communications/ Defines interfaces for adaptive upload functionality
tests/ Comprehensive test coverage for adaptive upload functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

// Start upload workers
for (var i = 0; i < 2; i++)
// Start upload workers (adaptive)
for (var i = 0; i < _adaptiveUploadController.CurrentParallelism; i++)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: if I understand correctly, the parallelism won't change for a given file. Besides, if two files are uploaded at the same time, the theorical parallelism will be doubled.
I think that it is not what has to be done: Parallelism must be able to change during file upload, and it must be shared across all uploads.

@sonarqubecloud
Copy link

@Ghass-M Ghass-M merged commit dab18a3 into feature/improve-upload-management Aug 27, 2025
21 checks passed
@Ghass-M Ghass-M deleted the feature/improve-upload-management-adaptive-controller branch August 27, 2025 09:40
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