Skip to content

Conversation

@andrewjl-mux
Copy link
Contributor

@andrewjl-mux andrewjl-mux commented Jul 6, 2023

Currently the file size is stored in ChunkedFile, this happens as a side effect of opening the file. This PR removes the stored property and retrieves the file size from FileManager directly whenever it is needed. The intention of this change is to reduced duplicate state between the SDK and FileManager as well as reduce the number of possible state permutations the SDK can take on.

@andrewjl-mux andrewjl-mux changed the base branch from main to releases/v0.4.0 July 6, 2023 18:53
@andrewjl-mux andrewjl-mux requested a review from a team July 6, 2023 18:54
Copy link
Collaborator

@daytime-em daytime-em left a comment

Choose a reason for hiding this comment

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

I really love this idea! Maybe the rest of the code that relies on fileSize could also benefit from this approach

Copy link
Contributor

@cjpillsbury cjpillsbury left a comment

Choose a reason for hiding this comment

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

Added some thoughts.

@andrewjl-mux andrewjl-mux changed the title Fetch file size without side effects Remove stored file size property from ChunkedFile Jul 6, 2023
@andrewjl-mux
Copy link
Contributor Author

Updated PR title and description so its more apparent what's changing vs not changing

Store fileURL inside ChunkedFile when an active handle is available

Simplify state checking inside ChunkedFile

Prevent potential crash due to overflow

Remove SIZE_UNKNOWN

Keep a reference to FileManager for eventual dependency injection when
initializing ChunkedFile
Copy link
Collaborator

@daytime-em daytime-em left a comment

Choose a reason for hiding this comment

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

LGTM I just had a question about swift practices

# Conflicts:
#	Sources/MuxUploadSDK/Upload/ChunkedFileUploader.swift
Copy link
Collaborator

@daytime-em daytime-em left a comment

Choose a reason for hiding this comment

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

Thanks for the discussion!

@andrewjl-mux
Copy link
Contributor Author

Just force pushed because the new file wasn't picked up earlier, merging now.

@andrewjl-mux andrewjl-mux merged commit 8c175d5 into releases/v0.4.0 Jul 6, 2023
@andrewjl-mux andrewjl-mux deleted the ajlb/file-size branch July 6, 2023 23:06
andrewjl-mux added a commit that referenced this pull request Jul 19, 2023
Store fileURL inside ChunkedFile when an active handle is available

Simplify state checking inside ChunkedFile

Prevent potential crash due to overflow

Remove SIZE_UNKNOWN

Keep a reference to FileManager for eventual dependency injection when
initializing ChunkedFile

Add extension method to work around Swift compiler buffoonery
cjpillsbury pushed a commit that referenced this pull request Jul 20, 2023
Store fileURL inside ChunkedFile when an active handle is available

Simplify state checking inside ChunkedFile

Prevent potential crash due to overflow

Remove SIZE_UNKNOWN

Keep a reference to FileManager for eventual dependency injection when
initializing ChunkedFile

Add extension method to work around Swift compiler buffoonery
andrewjl-mux added a commit that referenced this pull request Jul 20, 2023
Store fileURL inside ChunkedFile when an active handle is available

Simplify state checking inside ChunkedFile

Prevent potential crash due to overflow

Remove SIZE_UNKNOWN

Keep a reference to FileManager for eventual dependency injection when
initializing ChunkedFile

Add extension method to work around Swift compiler buffoonery
@github-actions github-actions bot mentioned this pull request Jul 20, 2023
andrewjl-mux added a commit that referenced this pull request Jul 20, 2023
Store fileURL inside ChunkedFile when an active handle is available

Simplify state checking inside ChunkedFile

Prevent potential crash due to overflow

Remove SIZE_UNKNOWN

Keep a reference to FileManager for eventual dependency injection when
initializing ChunkedFile

Add extension method to work around Swift compiler buffoonery
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.

4 participants