Port uc_authorizenet to D8

Patches need to be made relative to the Ubercart directory, so for example in your patch

diff --git a/uc_authorizenet.module b/uc_authorizenet.module
index c994b54..d6508da 100644
--- a/uc_authorizenet.module
+++ b/uc_authorizenet.module

should really be

diff --git a/payment/uc_authorizenet/uc_authorizenet.module b/payment/uc_authorizenet/uc_authorizenet.module
index c994b54..d6508da 100644
--- a/payment/uc_authorizenet/uc_authorizenet.module
+++ b/payment/uc_authorizenet/uc_authorizenet.module

etc.

These patches are a step in the right direction, but they are far from complete.

@sarjeet.singh: Why didn't you include the changes from #4 in your patch? If they are needed then they should be included, otherwise they will be overlooked. If they are not needed then you need to explain why.

Here are my comments about the patches so far:

1. Missing changes: This is only a partial list of some obviously missing things I noticed by skimming through the code just now:
1. Replace the remaining variable_get() with configuration variable and remove the variable_get() shim defined in uc_authorizenet.module.
2. Remove superglobals https://www.drupal.org/node/2150267
3. Use string functions from the Unicode utility class, e.g. Unicode::strtoupper(), instead of the PHP equivalents which don't support unicode.
4. Remove exit() call.
5. Move _uc_authorizenet functions into a helper class, modify the function names appropriately.
6. Explicitly mention which features you have tested and which you have not.
2. Functional changes: Changes to functionality shouldn't be part of the port - that should be a separate feature request. For example, you add 'http_errors' => false, which is a change from what we had.
3. Interdiff: When you post a new patch, also post an interdiff so we can see how your patch is different from the previous one - that way we don't have to do a complete review every time a patch is posted. See https://www.drupal.org/documentation/git/interdiff
4. Coding standards: Do not introduce new coding standards violations. Any line of code you modify or add should conform to the current coding standards. See https://www.drupal.org/docs/develop/standards/coding-standards . Don't mix unrelated coding standards fixes into the patch for the port - those can be fixed in a separate issue - here you should only fix lines you modify or add. You can click on the green test results "PHP 5.5 & MySQL 5.5, D8.4 159 pass" next to your patch to see the coding standards analysis for your patch.

I do think that the existing uc_authorizenet code is almost working, but working and ported completely are two different things. Because I don't have an Authorize.Net account, I can't do any detailed testing of the module - that's what is needed most! It's very important to test every feature of the module, even ones that you don't currently use.

Thanks @Nathaniel! It's looking a lot better, but the main thing is simply having someone like you who can test it and confirm it works.

There're still a lot of small things I see that need to be done, but I think they can be done in stages - i.e. commit the big patch then fix the little things later. The little things are things like service injection, coding standards, and documentation. If you're willing to help test these patches that would help a lot to get things finished.

The only big thing I see is that all the utility functions should be put into a separate class - NOT into the AuthorizeNet plugin. It's just way too messy to put them all into one class. But the bigger reason is that that stuff should really be refactored to use SimpleXML, and having them separated will make the refactoring easier. The existing code was initially written for Drupal 5 by people who did not know PHP very well, and when PHP was much worse at supporting OO code than it is now (still not very good, but better ...) The only good thing to say for the existing code is that it seems to work. But doing our own XML parsing and passing around undocumented arrays is really a very poor way to do things. It's not robust because the parsing doesn't, for example, escape special characters in the text strings (what if someone's name has a '<' in it? Then all the parsing would break), it doesn't handle multibyte strings, it's hard to read and understand therefore easy to hide bugs. It's also not very efficient. Using the built-in SimpleXML classes will greatly reduce our code, make the remaining code simple to read and more robust and maintainable.

I'm willing to do the XML stuff if you're not familiar with SimpleXML, as long as you will test any modifications I make.

But first, lets fix a few things from the patch in #10.

AuthorizeNetController.php:

1. You shouldn't inject the config.factory service because it's not used anywhere.
2. For the request_stack service, you're always accessing it with $this->requestStack->getCurrentRequest()->request->get(), which is pretty long and unnecessary. Instead of saving the service in the $requestStack variable, change the constructor to save $request_stack->getCurrentRequest()->request in a *protected* class variable named $request. Then you can use $this->request->get() everywhere else and it become much more readable.
3. We should take this opportunity to rename the uc_auth_arb_payment hook to uc_authorizenet_arb_payment. And this hook should be documented in uc_authorizenet.api.php (it is not currently)

AuthorizeNet.php:

1. Using the messenger service is not sufficient. I think the check on encryption setup needs to be done in validateConfigurationForm()
2. I don't like seeing isset() used anywhere - if you need this, that means some variable isn't getting initialized properly. Configuration variables should always have default values, so I would prefer to have defaults set in defaultConfiguration() rather than testing with isset()
3. It looks like you copied the changes @sarjeet.singh made in the \Drupal::httpClient() calls. I don't know why those changes were made, but I think they are a step backwards in terms of clarity. I would appreciate it if you restored those to the original form (using post() etc. instead of request()).
4. Move the functions formerly known as _uc_authorizenet_ ... into a separate class. I don't care what you name it, as it will be refactored/renamed in the conversion to SimpleXML.

uc_authorizenet.module:

1. Remove the constant definitions - they should be const variables in the AuthorizeNet.php file. What's more, since there is nothing left in uc_authorizenet.module, you can remove uc_authorizenet.module entirely. D8 does not require a module to have a .module file (only .info.yml is required).

General:

1. Check coding standards. The easiest way to do that is post the patch so the testbot can look at it, then look at the testbot results and fix any mentioned coding standards in the lines of code you've added.
2. Code that used to be in uc_authorizenet.module was using t(), but when you move that code into a class you should check to see if you can change this to $this->t(). Many base classes, such as ControllerBase, use the StringTranslationTrait.

I'm sure there's more, but this would be a great start. Let me know if you want me to work on the XML stuff - I can get that done over the next week if you are willing to test it.

Thanks @Nathaniel. It will take me a day or two to get around to reviewing this in depth. My initial glance shows that you addressed all the things I mentioned (Thanks!). If all looks good after the detailed review I will probably just commit this then submit a new patch to change the helper class to SimpleXML. The only thing that I noticed so far is that you tend to private a lot, whereas in Drupal we discourage the use of private, for various reasons. IMO private should be used in OO code only if you have a really really good reason why a subclass should not touch that variable or method. If you don't have a good reason, protected should be used.