Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support IO-like objects efficiently #41

Merged
merged 1 commit into from
Aug 2, 2016

Conversation

janko
Copy link
Contributor

@janko janko commented Jul 25, 2016

Shrine works with the abstraction of IO-like objects. For an object to be "IO-like" it proved to be enough that it responds to five IO methods: #read, #size, #rewind, #eof?, and #close.

However, MimeMagic expects the IO object to also respond to #seek (and also #binmode). If it doesn't respond to #seek, MimeMagic will currently read the whole file into memory. This will cause problems with large files and remote files.

Fortunately, the functionality of io.seek(n) can actually be produced using only the five methods mentioned above:

io.rewind
io.read(n)

This PR switches MimeMagic to use only these methods, allowing more generic IO-like objects.

end
end
MimeMagic.by_magic(StringableObject.new).should.equal 'application/mimemagic-test'
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you remove that test?

Copy link
Contributor Author

@janko janko Jul 26, 2016

Choose a reason for hiding this comment

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

You're right, there was no reason, I added them back, but changed ReadableObj to IOObject. Even though MimeMagic currently uses only #read and #rewind, it might use other IO methods in the future, and I think it's good to set up the complete "IO contract" upfront.

Previously if we passed in an IO object which doesn't respond to #seek,
MimeMagic would read the whole IO into memory. And this is exactly the
type of objects that the Shrine file upload library deals with.

However, we can also "seek" by blank-reading that many bytes, and always
rewinding to the beginning, which is what we've implemented in this
change.

This change also doesn't require IOs to respond to #binmode.
@janko
Copy link
Contributor Author

janko commented Jul 29, 2016

@minad Do you think we can merge this and release a new version? I would like to be able to have this available for Shrine users 😃

@minad
Copy link
Collaborator

minad commented Aug 2, 2016

Hi, sure go forward with it. I added you as a collaborator!

@janko janko merged commit 34a349b into mimemagicrb:master Aug 2, 2016
@janko janko deleted the io-like-objects branch August 2, 2016 18:28
@janko
Copy link
Contributor Author

janko commented Aug 2, 2016

Just wanted to check with you. Awesome! Do you think you could release a new version? I think it is still a patch version, as nothing new was added, just performance improvements.

@minad
Copy link
Collaborator

minad commented Aug 2, 2016

done :)

@janko
Copy link
Contributor Author

janko commented Aug 2, 2016

Thanks!

@mimemagicrb mimemagicrb locked as resolved and limited conversation to collaborators Mar 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants