Fortis: Initial implementation #5349

Heavyblade · 2024-11-27T00:01:51Z

Summary:

Fortis Gateway initial implementation with support for:

Authorize
Purchase
Void
Capture
Refund
Credit
Store
Unstore

Net::HTTP Refinement:

Context:

There is an issue with the Fortis API because their back-end returns an "invalid credentials" error if the HTTP Request headers are not in lower-case format, but Ruby by default follows the HTTP 1.1 RFC 2616 on how to handle headers; Ruby camellize all headers from something like 'user-api-key' to 'User-Api-Key', that is performed by Net::HTTPHeader#capitalize, so to deal with that we had to find a way to force net/http to send lower-case headers.

Enter Refinements:

So with Ruby Refinements, we can monkey patch net/http but only in the context of ActiveMerchant requests without side-effects on other areas of the system, two changes were introduced:

The CaseSensitiveHeaders class, an alias for a hash, allows us to indicate in the Fortis adapter that we will need case-sensitive headers.
The net/http refinement => that creates one HTTP::Request class per HTTP method, injecting a module that will overwrite the #capitalize method, it also overwrites the post, put, patch and delete methods to use the custom requests only when the headers sent to the Connection class are CaseSensitiveHeaders.

Remote Test:

Finished in 114.375184 seconds.
22 tests, 53 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
0.19 tests/s, 0.46 assertions/s

Unit Tests:

Finished in 68.455283 seconds.
6138 tests, 80277 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

RuboCop:

807 files inspected, no offenses detected

javierpedrozaing

Awesome job 🚀

javierpedrozaing · 2024-11-28T21:01:23Z

Rakefile

@@ -31,6 +31,7 @@ RuboCop::RakeTask.new

 namespace :test do
  Rake::TestTask.new(:units) do |t|
+    ENV['RUNNING_UNIT_TESTS'] = 'true'


Why is this ENV needed ❓

Oh, this one is a workaround for preventing Net::HTTP refinement from having some effect on unit tests mocs on test/unit/connection_test.rb

javierpedrozaing · 2024-11-28T21:26:56Z

lib/active_merchant/billing/gateways/fortis.rb

+        331 => 'Credit/Debit/Refund ach Charged Back'
+      }
+
+      REASON_MAPPING = {


was the mapping for status and reason needed due the Fortis response does not return a readable message, or was it for another specific reason ❓

Most of the times Fortis returns an status_mapping and a reason_mapping but not the actual message

javierpedrozaing · 2024-11-28T21:29:27Z

lib/active_merchant/billing/gateways/fortis.rb

+      end
+
+      def commit(action, post, method = :post, options = {})
+        add_location_id(post) if post.present?


what is the advantage of sending the location_id in the post ❓

Is an optional normally that is usefull to identify the request source (Fortis customer normally are shops with card machines or terminals) and for the store transaction is a must.

javierpedrozaing · 2024-11-28T21:33:32Z

lib/active_merchant/connection.rb

@@ -76,7 +76,7 @@ def request(method, body, headers = {})
              http.get(endpoint.request_uri, headers)
            when :post
              debug body
-              http.post(endpoint.request_uri, body, RUBY_184_POST_HEADERS.merge(headers))
+              http.post(endpoint.request_uri, body, headers.reverse_merge!(RUBY_184_POST_HEADERS))


Is this change related to the HTTP version ❓

Exactly, see:

the headers object is a CaseSensitiveHeaders object that acts like a Hash, the merge method called on RUBY_182_POST_HEADERS hash will return a new Hash not a CaseSensitveHeaders that I need in the refinement.

Why using reverse merge? Is order of these headers also important?

lib/active_merchant/net_http_ssl_connection.rb

lib/active_merchant/posts_data.rb

naashton

Most of my review is around the changes to the http modules to accommodate this gateways quirkiness. It seems like the implementation should leave the existing gateways unaffected, but did you run any of the remote tests against any other gateways to see if they yielded any different results?

naashton · 2024-12-02T17:39:14Z

lib/active_merchant/billing/gateways/fortis.rb

+      }
+
+      def initialize(options = {})
+        requires!(options, :user_id, :user_api_key, :developer_id, :location_id)


I think based on our discussion, we probably can remove location_id and make it an optional class field.

naashton · 2024-12-02T20:24:40Z

lib/active_merchant/connection.rb

@@ -76,7 +76,7 @@ def request(method, body, headers = {})
              http.get(endpoint.request_uri, headers)
            when :post
              debug body
-              http.post(endpoint.request_uri, body, RUBY_184_POST_HEADERS.merge(headers))
+              http.post(endpoint.request_uri, body, headers.reverse_merge!(RUBY_184_POST_HEADERS))


Why using reverse merge? Is order of these headers also important?

naashton · 2024-12-02T20:34:51Z

lib/active_merchant/net_http_ssl_connection.rb

+    private :capitalize
+  end
+
+  class CaseSensitivePost < Net::HTTP::Post; prepend InnocuousCapitalize; end


Is the prepend intended to give precedence to the existing behavior, which is capitalizing header fields?

reponses:

no because of order, but because of classes see => hash_object.merge!(case_sensitve_header_hash), en-up being a plain ruby Hash, but case_sensitve_header_hash.merge!(hash_object) updates CaseSensitiveHeaders instant but is still that class, so I can differentiate in the Refinement.

yes exactly that

lib/active_merchant/billing/gateways/fortis.rb

Summary: ------------------------------ Fortis Gateway intial implementation with support for: - Authorize - Purchase - Void - Capture - Refund - Credit - Store - Unstore Remote Test: ------------------------------ Finished in 114.375184 seconds. 22 tests, 53 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed 0.19 tests/s, 0.46 assertions/s Unit Tests: ------------------------------ Finished in 68.455283 seconds. 6138 tests, 80277 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed RuboCop: ------------------------------ 807 files inspected, no offenses detected

Heavyblade force-pushed the fortis_base_gateway_adapter branch 4 times, most recently from 77daa32 to c643967 Compare November 27, 2024 15:13

Heavyblade requested review from javierpedrozaing and naashton November 27, 2024 15:16

Heavyblade force-pushed the fortis_base_gateway_adapter branch from c643967 to 20b9b8b Compare November 27, 2024 16:19

javierpedrozaing approved these changes Nov 28, 2024

View reviewed changes

Heavyblade force-pushed the fortis_base_gateway_adapter branch from 20b9b8b to 3142cd0 Compare November 29, 2024 21:58

naashton reviewed Dec 2, 2024

View reviewed changes

Heavyblade force-pushed the fortis_base_gateway_adapter branch from 3142cd0 to 2f67c66 Compare December 3, 2024 15:24

Buitragox reviewed Dec 3, 2024

View reviewed changes

lib/active_merchant/billing/gateways/fortis.rb Show resolved Hide resolved

Heavyblade force-pushed the fortis_base_gateway_adapter branch from 38324b9 to 50f3987 Compare December 4, 2024 14:26

naashton approved these changes Dec 4, 2024

View reviewed changes

Heavyblade merged commit 599f2cc into master Dec 4, 2024
5 checks passed

Heavyblade deleted the fortis_base_gateway_adapter branch December 4, 2024 14:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fortis: Initial implementation #5349

Fortis: Initial implementation #5349

Heavyblade commented Nov 27, 2024 •

edited

Loading

javierpedrozaing left a comment

javierpedrozaing Nov 28, 2024

Heavyblade Dec 2, 2024

javierpedrozaing Nov 28, 2024

Heavyblade Dec 2, 2024

javierpedrozaing Nov 28, 2024

Heavyblade Dec 2, 2024

javierpedrozaing Nov 28, 2024

Heavyblade Dec 2, 2024

naashton Dec 2, 2024

naashton left a comment

naashton Dec 2, 2024

naashton Dec 2, 2024

naashton Dec 2, 2024

Heavyblade Dec 2, 2024

Fortis: Initial implementation #5349

Fortis: Initial implementation #5349

Conversation

Heavyblade commented Nov 27, 2024 • edited Loading

Summary:

Net::HTTP Refinement:

Context:

Enter Refinements:

Remote Test:

Unit Tests:

RuboCop:

javierpedrozaing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naashton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Heavyblade commented Nov 27, 2024 •

edited

Loading