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 JSON-RPC API support #2273

Merged
merged 7 commits into from
Aug 15, 2023
Merged

Added JSON-RPC API support #2273

merged 7 commits into from
Aug 15, 2023

Conversation

fballiano
Copy link
Contributor

This PR adds support for JSON RPC protocol for API calls.

Manual testing scenarios (*)

I tested these APIs with postman using this "body" for the login method
Schermata 2022-07-03 alle 15 06 35

and this "body" for the "call" method
Schermata 2022-07-03 alle 15 09 28

@fballiano
Copy link
Contributor Author

PHPStan fails for reasons unrelated to this PR.

empiricompany
empiricompany previously approved these changes Mar 30, 2023
@fballiano fballiano changed the base branch from 20.0 to main April 4, 2023 17:33
@fballiano fballiano dismissed stale reviews from empiricompany and FredericMartinez April 4, 2023 17:33

The base branch was changed.

empiricompany
empiricompany previously approved these changes Apr 4, 2023
kiatng
kiatng previously approved these changes Apr 16, 2023
Copy link
Collaborator

@kiatng kiatng left a comment

Choose a reason for hiding this comment

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

I have used a variation of this which enabled soap_v2 style calling.

@fballiano fballiano dismissed stale reviews from kiatng and empiricompany via 6dbf96d May 22, 2023 10:36
Copy link
Collaborator

@kiatng kiatng left a comment

Choose a reason for hiding this comment

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

My test script:

    public function jsonrpcAction()
    {
        $result = [];
        $url = Mage::getStoreConfig('muze/stars/url');
        $client = new Krixon_JsonRpc_Client($url);

        $sessionId = Mage::getSingleton('customer/session')->getData('jsonsessionKey');
        if (!$sessionId) {
            $apiUser = Mage::getStoreConfig('muze/stars/user');
            $apiKey = Mage::getStoreConfig('muze/stars/key');
            $session = $client->call('login', [$apiUser, $apiKey]);
            if ($session instanceof stdClass) {
                if (property_exists($session, 'result')) {
                    $sessionId = $session->result;
                    Mage::getSingleton('customer/session')->setData('jsonsessionKey', $sessionId);
                } else {
                    Mage::getSingleton('customer/session')->unsetData('jsonsessionKey');
                    return $this->_echo($session, 'cannot login');
                }
            } else {
                return $this->_echo($session, 'unexpected response');
            }
            $result['new_session'] = $sessionId;
        } else {
            $result['session'] = $sessionId;
        }

        /** @var stdClass $std */
        $std = $client->call('call', [$sessionId, 'magento.info', []]);
        if (property_exists($std, 'error')) {
            if ($std->error->code === -32002) {
                Mage::getSingleton('customer/session')->unsetData('jsonsessionKey');
            }
        }
        $result['magento.info'] = $std;

        $this->_echo($result);
    }

Result:

 array(2) {
  ["new_session"] => string(32) "a3253324b025fcd5789659c2f75aaa06"
  ["magento.info"] => object(stdClass)#168 (3) {
    ["result"] => object(stdClass)#81 (3) {
      ["magento_edition"] => string(9) "Community"
      ["magento_version"] => string(7) "1.9.4.5"
      ["openmage_version"] => string(7) "20.0.16"
    }
    ["id"] => string(13) "64da3c21b96ef"
    ["jsonrpc"] => string(3) "2.0"
  }
}

@fballiano fballiano merged commit 9f331f6 into OpenMage:main Aug 15, 2023
15 checks passed
@fballiano fballiano deleted the jsonrpc branch August 15, 2023 09:36
@fballiano fballiano changed the title JSON-RPC API support Added JSON-RPC API support Aug 15, 2023
@medeirosleandro
Copy link

Please credit the source repository @gamuzatech

@eneiasramos
Copy link
Contributor

This implementation is very similar to my module:

https://github.com/gamuzatech/gamuza_jsonapi-magento

@medeirosleandro
Copy link

This implementation is very similar to my module:

https://github.com/gamuzatech/gamuza_jsonapi-magento

Please give due credit to the source repository @gamuzatech, there was time for research and development so that I could contribute to the community

@fballiano
Copy link
Contributor Author

wow, zero comments in 1.1 years and 3 comments accusing me of stealing in 3 minutes. I'll put my answer in points for easier quoting

  1. I didn't know about your module until reading your comments, this pr is more than 1 year old so I don't remember everything in detail but I recall getting the idea from a stackoverflow question I stumbled upon while looking for something else.
  2. Your module is much more complex than this PR but most of all is GPL2. I've posted many times agains using GPL code in openmage because of it's viral nature and because it's considered not compatible with OSL, so I wouldn't have ever read your code
  3. with a quick google seach I've found at least one more implementation of the same feature, which is actually 1 year older than yours and much simpler, in a way much more similar to this PR than yours
  4. the implementation of this PR is extremely easy, there's barely any code and you can check that it's directly a copy paste of the Xmlrpc classes, that's why I added the original "Copyright (c) 2006-2020 Magento", because the files were copied from there.
  5. the other public implementations of the same feature are also 99% copying the boilerplate from the original Xmlrpc from the core, there are maybe 10 lines which could be considered different but that 1% difference is trivial code, I mean... "Zend_Json_Exception" instead of "Zend_XmlRpc_Server_Exception", "$this->_jsonRpc" instead of "$this->_xmlRpc", that's not real code in this PR.
  6. this PR is so much a copy of the core's classes that it still has "array()" notation instead of "[]" because that was converted after this PR was opened and I forgot to convert it here too.

@colinmollenhour @ADDISON74 @kiatng @elidrissidev @Flyingmana what do you think we should do here?

@ADDISON74
Copy link
Collaborator

My opinion is that your PR solves an issue reported in OpenMage. It was suggested by a person I have not met so far in discussions that another author should be credited for this PR. The source shown does not resemble your implementation, I doubt that you analyzed it because you had to spend time to see if it solves the issue, then adapted to OpenMage not as a module but directly in the existing code. From a legal perspective, if we look at the copyright applied to the source, the situation is resolved.

Before coming with legal arguments, I ask kindly the person @medeirosleandro who made the accusation that the code is taken from another repository to come with clear evidence in this regard. If it is only about the fact that there is someone else with an implementation, then the initial statement was wrong because we have to discuss about stealing code. Then we decide whether to credit the author or not.

@fballiano
Copy link
Contributor Author

fballiano commented Aug 15, 2023

this PR contains 3 files

  1. https://github.com/gamuzatech/gamuza_jsonapi-magento/blob/master/app/code/local/Gamuza/JsonApi/controllers/JsonController.php doesn't really seem like the one in this PR, there's 1 line which looks the same and that's because it's exactly equal to the one in the core https://github.com/OpenMage/magento-lts/blob/main/app/code/core/Mage/Api/controllers/XmlrpcController.php
  2. https://github.com/gamuzatech/gamuza_jsonapi-magento/blob/master/app/code/local/Gamuza/JsonApi/Model/Server/Adapter/Json.php, it's obviously similar to mine because, again, it's exactly the same as https://github.com/OpenMage/magento-lts/blob/main/app/code/core/Mage/Api/Model/Server/Adapter/Xmlrpc.php
  3. 3rd file is a 3 lines xml config file, which again, the 3 lines are exactly copied from the 3 lines right above.

@ADDISON74
Copy link
Collaborator

@fballiano - You don't have to explain, but the one who made the accusation, which seems serious to me.

@colinmollenhour
Copy link
Member

I don't see any evidence that the gamuzatech repo was used as a source - same idea, simple feature, same outcome. Given that you're implementing an interface there is basically only one way to do it and JSONRPC is an open standard..

If @medeirosleandro or @eneiasramos want to push it further I think they need to make a stronger case than just having done it first. I made a JSONRPC adapter myself for a private project back in February of 2013 which pre-dates the Gamuza one - it really wasn't hard, will gladly share it if needed but I think that is not necessary.

@elidrissidev
Copy link
Member

I'd say it resembles the core Xmlrpc code more than the other repo, the yoda conditions and long array syntax refactoring was done after this PR was created.

Besides, there's only so many ways to implement such functionality, so having some resemblance between them isn't out of the ordinary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Api PageRelates to Mage_Api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants