Merge pull request #16 from violetviolinist/no-transforms-fix

add test case for no transformations URL + improve code coverage

Author: imagekitio

.github/workflows/test.yml

@@ -8,13 +8,22 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-    - uses: actions/checkout@v2
-    - name: Set up Ruby 2.6
-      uses: actions/setup-ruby@v1
-      with:
-        ruby-version: 2.6.x
-    - name: Build and test with Rake
-      run: |
-        gem install bundler
-        bundle install --jobs 4 --retry 3
-        bundle exec rake
+      - uses: actions/checkout@v2
+      - name: Set up Ruby 2.6
+        uses: actions/setup-ruby@v1
+        with:
+          ruby-version: 2.6.x
+      - name: Build and test with Rake
+        run: |
+          gem install bundler
+          bundle install --jobs 4 --retry 3
+          bundle exec rake
+      # - name: Upload code coverage reports to codecov
+      #   uses: codecov/codecov-action@v1
+      #   with:
+      #     token: 58fd7bff-e88e-4f23-b940-811b12f20dcf # not required for public repos
+      #     # file: ./coverage.xml # optional
+      #     # files: ./coverage1.xml,./coverage2.xml # optional
+      #     # flags: unittests # optional
+      #     # name: codecov-umbrella # optional
+      #     # fail_ci_if_error: true # optional (default = false)

.gitignore

@@ -1,6 +1,7 @@
 .bundle/
 log/*.log
 pkg/
+coverage/
 test/dummy/db/*.sqlite3
 test/dummy/db/*.sqlite3-journal
 test/dummy/db/*.sqlite3-*

Gemfile

@@ -17,4 +17,7 @@ gemspec
 gem "minitest", "~> 5.0"
 # gem "rake", "~> 12.0"
 # gem "rest-client", "~>2.1"
-gem "rspec"
+gem "rspec"
+
+gem 'codecov', require: false, group: 'test'
+gem 'webmock', require: false, group: 'test'

lib/imagekit/constants/errors.rb

@@ -69,3 +69,9 @@
                        'help': "Please pass two pHash values",}
 UNEQUAL_STRING_LENGTH = {'message': "Unequal pHash string length",
                          'help': "For distance calculation, the two pHash strings must have equal length",}
+
+MISSING_PRIVATE_KEY = "ImageKit private key missing"
+
+MISSING_PUBLIC_KEY = "ImageKit public key missing"
+
+MISSING_URL_ENDPOINT = "ImageKit URL Endpoint missing. Default URL Endpoint: https://ik.imagekit.io/<YOUR_IMAGEKIT_ID>/"

lib/imagekit/file.rb

@@ -130,4 +130,4 @@ def validate_upload_options(options)
     end
     request_formatter(options)
   end
-end
+end

lib/imagekit/imagekit.rb

@@ -16,6 +16,17 @@ class ImageKitClient
     attr_reader :file
 
     def initialize(private_key, public_key, url_endpoint, transformation_pos = nil, options = nil)
+
+      unless(private_key.is_a?(String) && private_key.to_s.strip.length != 0)
+        raise ArgumentError, MISSING_PRIVATE_KEY
+      end
+      unless(public_key.is_a?(String) && public_key.to_s.strip.length != 0)
+        raise ArgumentError, MISSING_PUBLIC_KEY
+      end
+      unless(url_endpoint.is_a?(String) && url_endpoint.to_s.strip.length != 0)
+        raise ArgumentError, MISSING_URL_ENDPOINT
+      end
+
       @private_key = private_key
       @public_key = public_key
       @url_endpoint = url_endpoint
@@ -92,8 +103,7 @@ def get_remote_file_url_metadata(remote_file_url = "")
     def phash_distance(first, second)
       # Get hamming distance between two phash(image hash) to check
       # similarity between images
-
-      unless first && second
+      if first.to_s.strip == "" || second.to_s.strip == ""
         raise ArgumentError, Error::MISSING_PHASH_VALUE
       end
       hamming_distance(first, second)
@@ -104,5 +114,4 @@ def get_authentication_parameters(token = nil, expire = nil)
       get_authenticated_params(token, expire, @ik_req.private_key)
     end
   end
-end
-
+end

lib/imagekit/resource.rb

@@ -38,11 +38,12 @@ def request(method, url, headers = nil, payload = nil)
                                      headers: headers,
                                      payload: payload).execute
 
-
-      if resp.code == 404
-        raise RestClient::ExceptionWithResponse
-      elsif (resp.code >= 200) && (resp.code < 204)
-        response[:response] = JSON.parse(resp.body.to_s)
+      if (resp.code >= 200) && (resp.code < 204)
+        if (resp.headers[:content_type] == "application/json")
+          response[:response] = JSON.parse(resp.body.to_s)
+        else
+          raise =RestClient::ExceptionWithResponse
+        end
       elsif resp.code == 204
         response[:response] = {'success': true}
       end

lib/imagekit/url.rb

@@ -70,8 +70,6 @@ def build_url(options)
         parts=part.split("=")
         if parts.length==2
           query_params[parts[0]]=parts[1]
-        else
-          query_params[parts[0]]=""
         end
       end
     end
@@ -94,25 +92,30 @@ def build_url(options)
     result_url_hash[:path] = result_url_hash[:path].chomp("/")
     result_url_hash[:scheme] ||= "https"
 
+    query_param_arr = []
+    query_param_arr.push("ik-sdk-version=ruby-"+Imagekit::Sdk::VERSION)
+    query_params.each do |key, value|
+      if value.to_s == ""
+        query_param_arr.push(key.to_s)
+      else
+        query_param_arr.push(key.to_s + "=" + value.to_s)
+      end
+    end
+
+    query_param_str = query_param_arr.join("&")
+    result_url_hash[:query] = query_param_str
 
     # Signature String and Timestamp
     # We can do this only for URLs that are created using urlEndpoint and path parameter
     # because we need to know the endpoint to be able to remove it from the URL to create a signature
     # for the remaining. With the src parameter, we would not know the "pattern" in the URL
-    query_param_arr = []
-    query_param_arr.push("ik-sdk-version=ruby-"+Imagekit::Sdk::VERSION)
     if options[:signed] && !(options[:src])
       intermediate_url = result_url_hash.fetch(:scheme, "") + "://" + result_url_hash.fetch(:host, "") + result_url_hash.fetch(:path, "")
       if result_url_hash[:query]!=nil && result_url_hash[:query]!=""
         intermediate_url += result_url_hash.fetch(:query, "")
       end
     end
-    query_params.each do |key, value|
-      query_param_arr.push(key.to_s + "=" + value.to_s)
-    end
 
-    query_param_str = query_param_arr.join("&")
-    result_url_hash[:query] = query_param_str
     url=hash_to_url(result_url_hash)
     if options[:signed]
       private_key = options[:private_key]

test/imagekit/calculation_test.rb

@@ -1,5 +1,6 @@
-require "rspec/autorun"
 require_relative './helper'
+require "rspec/autorun"
+
 RSpec.describe "CalculationTest" do
 
     it "is_valid_hex_with_valid_hex" do
@@ -57,4 +58,10 @@
         expect(nil).not_to eq(result[:expire])
         expect('my_signature').to eq(result[:signature])
     end
+
+    it "get_authenticated_params_test_with_nil_private_key" do
+        allow(OpenSSL::HMAC).to receive(:hexdigest).and_return('my_signature')
+        result=get_authenticated_params('your_token', 1582269249, nil)
+        expect(result).to eq(nil)
+    end
 end

test/imagekit/file_test.rb

@@ -1,5 +1,5 @@
-require "rspec/autorun"
 require_relative './helper'
+require "rspec/autorun"
 
 RSpec.describe "FileUploadTest" do
   it "test_upload_with_valid_expected_success_without_tags_and_remote_url" do
@@ -124,6 +124,27 @@
 
   end
 
+  it "test_upload_fails_on_invalid_options" do
+    request_obj = double
+    allow(ImageKitRequest)
+      .to receive(:new)
+      .with(PRIVATE_KEY, PUBLIC_KEY, URL_ENDPOINT)
+      .and_return(request_obj)
+
+    allow(request_obj)
+      .to receive(:create_headers)
+      .and_return({})
+
+    allow(request_obj)
+      .to receive(:request){|method,url,headers,payload| @ac={method: method, url: url, headers: headers, payload:payload}}
+      .and_return({code: 401, message: nil})
+
+    SUT = ImageKitFile.new(request_obj)
+    expect{
+      SUT.upload("fake_file.jpg", "fake_name", { invalid_option: "invalid_option" })
+    }.to raise_error(ArgumentError)
+  end
+
   it "test_upload_fails_on_unauthenticated_request" do
     request_obj = double
     allow(ImageKitRequest)
@@ -504,9 +525,77 @@
     expect(resp[:code]).to eq(200)
     expect(resp[:body]).to eq(options)
   end
+
+  it "test_update_file_details_fails_missing_arguments" do
+    options = {}
+    request_obj = double
+    allow(ImageKitRequest)
+      .to receive(:new)
+      .with(PRIVATE_KEY, PUBLIC_KEY, URL_ENDPOINT)
+      .and_return(request_obj)
+
+    allow(request_obj)
+      .to receive(:create_headers)
+      .and_return({})
+
+    allow(request_obj)
+      .to receive(:request){|method,url,headers,payload| @ac={method: method, url: url, headers: headers, payload:payload}}
+      .and_return({code: 200, body: options})
+
+    SUT = ImageKitFile.new(request_obj)
+    expect { 
+      SUT.update_details("file_id", options)
+  }.to raise_error(ArgumentError)
+  end
+
+  it "test_update_file_details_fails_tags_not_an_array" do
+    options = {tags: "RANDOM_TEXT", "custom_coordinates": "10,10,100,100"}
+    request_obj = double
+    allow(ImageKitRequest)
+      .to receive(:new)
+      .with(PRIVATE_KEY, PUBLIC_KEY, URL_ENDPOINT)
+      .and_return(request_obj)
+
+    allow(request_obj)
+      .to receive(:create_headers)
+      .and_return({})
+
+    allow(request_obj)
+      .to receive(:request){|method,url,headers,payload| @ac={method: method, url: url, headers: headers, payload:payload}}
+      .and_return({code: 200, body: options})
+
+    SUT = ImageKitFile.new(request_obj)
+    expect { 
+      SUT.update_details("file_id", options)
+  }.to raise_error(ArgumentError)
+  end
+
+  it "test_update_file_details_fails_custom_coordinates_not_a_string" do
+    options = {"custom_coordinates": %w[random]}
+    request_obj = double
+    allow(ImageKitRequest)
+      .to receive(:new)
+      .with(PRIVATE_KEY, PUBLIC_KEY, URL_ENDPOINT)
+      .and_return(request_obj)
+
+    allow(request_obj)
+      .to receive(:create_headers)
+      .and_return({})
+
+    allow(request_obj)
+      .to receive(:request){|method,url,headers,payload| @ac={method: method, url: url, headers: headers, payload:payload}}
+      .and_return({code: 200, body: options})
+
+    SUT = ImageKitFile.new(request_obj)
+    expect { 
+      SUT.update_details("file_id", options)
+  }.to raise_error(ArgumentError)
+  end
 end
 
 
+
+
 RSpec.describe "TestBatchDelete" do
   it "test_batch_delete_fails_on_unauthenticated_request" do
     file_ids = %w[file_id_1 file_id_2]
@@ -605,7 +694,27 @@
 
     expect(@ac[:url]).to eq("https://api.imagekit.io/v1/metadata?url=http://example.com/fakefileurl")
     expect(resp[:code]).to eq(401)
+  end
+
+  it "test_get_metadata_from_remote_url_fails_on_blank_url" do
+    request_obj = double
+    allow(ImageKitRequest)
+      .to receive(:new)
+      .with(PRIVATE_KEY, PUBLIC_KEY, URL_ENDPOINT)
+      .and_return(request_obj)
+
+    allow(request_obj)
+      .to receive(:create_headers)
+      .and_return({})
 
+    allow(request_obj)
+      .to receive(:request){|method,url,headers,payload| @ac={method: method, url: url, headers: headers, payload:payload}}
+      .and_return({code: 401, body: {}})
+
+    SUT = ImageKitFile.new(request_obj)
+    expect {
+      SUT.get_metadata_from_remote_url("")
+  }.to raise_error(ArgumentError)
   end
 
   it "test_get_metadata_from_remote_url_succeeds" do