-
Notifications
You must be signed in to change notification settings - Fork 66
Need review about this #25
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,18 @@ class PKPass | |
|
|
||
| TYPES = ['boarding-pass', 'coupon', 'event-ticket', 'store-card', 'generic'] | ||
|
|
||
| # Require fields, meta programming for accessor | ||
| REQUIRED_FIELDS = %w(passTypeIdentifier teamIdentifier serialNumber organizationName formatVersion description) | ||
| REQUIRED_FIELDS.each do |accessor| | ||
| class_eval %Q{ | ||
| def #{accessor}= value | ||
| json = JSON.parse(@pass) | ||
| json['#{accessor}'] = value | ||
| @pass = json.to_json | ||
| end | ||
| } | ||
| end | ||
|
|
||
| def initialize pass | ||
| @pass = pass | ||
| @manifest_files = [] | ||
|
|
@@ -44,8 +56,13 @@ def create | |
| self.file.path | ||
| end | ||
|
|
||
| # Return a Tempfile containing our ZipStream | ||
| # Backward compatibility | ||
| def file(options = {}) | ||
| to_file options | ||
| end | ||
|
|
||
| # Return a Tempfile containing our ZipStream | ||
| def to_file options = {} | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No issues here. The unit tests should catch these in the regressions. |
||
| options[:file_name] ||= 'pass.pkpass' | ||
|
|
||
| temp_file = Tempfile.new(options[:file_name]) | ||
|
|
@@ -55,8 +72,13 @@ def file(options = {}) | |
| temp_file | ||
| end | ||
|
|
||
| # Return a ZipOutputStream | ||
| # Backward compatibility | ||
| def stream | ||
| to_stream | ||
| end | ||
|
|
||
| # Return a ZipOutputStream | ||
| def to_stream | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These should get caught in the regressions. |
||
| manifest, signature = build | ||
|
|
||
| outputZip manifest, signature | ||
|
|
@@ -69,9 +91,9 @@ def get_p12_cert_and_key | |
| key_hash[:cert] = OpenSSL::X509::Certificate.new File.read(Passbook.p12_certificate) | ||
| else | ||
| p12 = OpenSSL::PKCS12.new File.read(Passbook.p12_cert), Passbook.p12_password | ||
| key_hash[:key], key_hash[:cert] = p12.key, p12.certificate | ||
| key_hash[:key], key_hash[:cert] = p12.key, p12.certificate | ||
| end | ||
| key_hash | ||
| key_hash | ||
| end | ||
|
|
||
| def createSignature manifest | ||
|
|
@@ -88,6 +110,16 @@ def createSignature manifest | |
| return Base64.decode64(data) | ||
| end | ||
|
|
||
| def valid? | ||
| manifest = createManifest | ||
| begin | ||
| checkPass manifest | ||
| rescue | ||
| return false | ||
| end | ||
| return true | ||
| end | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should have a unit test around it. |
||
| private | ||
|
|
||
| def checkPass manifest | ||
|
|
@@ -96,13 +128,11 @@ def checkPass manifest | |
| raise 'Icon@2x missing' unless manifest.include?('icon@2x.png') | ||
|
|
||
| # Check for developer field in JSON | ||
| raise 'Pass Type Identifier missing' unless @pass.include?('passTypeIdentifier') | ||
| raise 'Team Identifier missing' unless @pass.include?('teamIdentifier') | ||
| raise 'Serial Number missing' unless @pass.include?('serialNumber') | ||
| raise 'Organization Name Identifier missing' unless @pass.include?('organizationName') | ||
| raise 'Format Version' unless @pass.include?('formatVersion') | ||
| REQUIRED_FIELDS.each do |require_field| | ||
| raise "#{require_field} mising" unless @pass.include?(require_field) | ||
| end | ||
| # Specific test | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This didn't initially have test coverage, but we should add that in for this. |
||
| raise 'Format Version should be a numeric' unless JSON.parse(@pass)['formatVersion'].is_a?(Numeric) | ||
| raise 'Description' unless @pass.include?('description') | ||
| end | ||
|
|
||
| def createManifest | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this.. Can you add a unit test or two around these?