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

Added base64 to UIImageExtensions #363

Merged
merged 8 commits into from Mar 9, 2017
Merged

Added base64 to UIImageExtensions #363

merged 8 commits into from Mar 9, 2017

Conversation

lfarah
Copy link
Collaborator

@lfarah lfarah commented Jan 13, 2017

fixes #355

Checklist

  • New Extension
  • New Test
  • Changed more than one extension, but all changes are related
  • Trivial change (doesn't require changelog)


/// EZSE: Returns base64 string
var base64: String {
return UIImageJPEGRepresentation(self, 1.0)!.base64EncodedString()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to test this ?

Copy link
Collaborator Author

@lfarah lfarah Jan 13, 2017

Choose a reason for hiding this comment

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

We could download an image with a link and then get the base 64 from a website like this: https://www.base64-image.de/

Copy link
Collaborator

@Khalian Khalian Jan 16, 2017

Choose a reason for hiding this comment

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

I would avoid network IO if I could. Lets keep a downloaded image in some testdata folder or something of that order. Or just test on charizard.png.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Khalian couldn't get the image to work. Keep getting nil

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lfarah Which image did you test it on ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The charizard one. Can you try to do it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes I was able to get it to work. My prototype code was :

public func testImageBase64() {
    let image = UIImage(contentsOfFile: "/workplace/arunavs/third_party/EZSwiftExtensions/charizard.png")
    print(image?.base64)
}

It gave a non nil base 64 encoded string.

Can you write a test(with the path formatted as a relative path) to assert that the output is not nil and also its of base 64 format (do we have an extension/foundation API for that?). I will approve this after that.

@EZSwiftExtensionsBot
Copy link

EZSwiftExtensionsBot commented Jan 13, 2017

1 Error
🚫 Please rebase to get rid of the merge commits in this PR
2 Messages
📖 Executed 181 tests, with 0 failures (0 unexpected) in 6.446 (6.775) seconds
📖 Executed 167 tests, with 0 failures (0 unexpected) in 4.849 (5.248) seconds

Generated by 🚫 Danger

@lfarah
Copy link
Collaborator Author

lfarah commented Jan 31, 2017

@Khalian I have no idea why the test is not working. Converted the .png to .jpg and the base64 string looks visually the same as the test str I got from this website

@Khalian
Copy link
Collaborator

Khalian commented Jan 31, 2017

You can avoid the merge commits by rebasing the branch locally and force pushing onto the remote branch (never force push on mainline).

@lfarah
Copy link
Collaborator Author

lfarah commented Jan 31, 2017

I can clean it later... Just wanna get this test working

@lfarah
Copy link
Collaborator Author

lfarah commented Jan 31, 2017

@vsouza can you help?

@Khalian
Copy link
Collaborator

Khalian commented Feb 5, 2017

As long as you get some base64 string, IMO its good enough a test.

@lfarah
Copy link
Collaborator Author

lfarah commented Feb 6, 2017

@Khalian what do you think?

let dataImage = try? Data(contentsOf: imageURL!)
let image = UIImage(data: dataImage!)

XCTAssertNotNil(image!.base64)
Copy link
Collaborator

@Khalian Khalian Feb 7, 2017

Choose a reason for hiding this comment

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

Lets add a stronger check here:-

if let decodedData = NSData(base64EncodedString: image!.base64, options: nil) {
    // Pass test case
} else {
   // Fail test case.     
}

@Khalian
Copy link
Collaborator

Khalian commented Feb 8, 2017

Dont forget to squash the commits into one.

@Khalian
Copy link
Collaborator

Khalian commented Feb 18, 2017

@piv199 @goktugyil What do you guys think ?

@Khalian
Copy link
Collaborator

Khalian commented Mar 8, 2017

@lfarah Can you replay your changes on existing master ?

Copy link
Owner

@Esqarrouth Esqarrouth left a comment

Choose a reason for hiding this comment

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

I don't really understand the code here but nothing looks weird so

@Khalian
Copy link
Collaborator

Khalian commented Mar 8, 2017

@goktugyil Its a base64 conversion of an image. Traditionally base64 conversions for different inputs (including binary encodings) are used to transmit information over the HTTP protocol.

@Esqarrouth
Copy link
Owner

I understand that part :D

@lfarah
Copy link
Collaborator Author

lfarah commented Mar 9, 2017

@goktugyil Basically base64 is the most used way to do image upload. I use it all the time on my freelance projects. There are a lot of changes because testing it was really shitty

@lfarah lfarah merged commit c6fcd96 into master Mar 9, 2017
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.

UIImage.base64
4 participants