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

Pass MWS keys without config file #97

Open
haffoudhi opened this issue Aug 6, 2016 · 23 comments
Open

Pass MWS keys without config file #97

haffoudhi opened this issue Aug 6, 2016 · 23 comments

Comments

@haffoudhi
Copy link

Is this possible ?
I need to pass different MWS keys every time, and one config file is not the correct way.
Is it possible to pass the keys directly without config files?

@Peardian
Copy link
Collaborator

Peardian commented Aug 8, 2016

I was under the impression that MWS Auth tokens were created once, at the same time as the Seller ID. Could you elaborate on your setup a bit? I need to know if this is an edge case or not before I decide on what to do about it.

@haffoudhi
Copy link
Author

I m building an application for multiple sellers, each seller provides his own MWS auth token and seller ID. The app must do multiple reports at the same time for multiple sellers, using one config file is not an option here. The solution i m using is creating a different config file for each user and passing path to config file in every call to "phpAmazonMWS". I would prefer a more simple way, where i can introduce to api keys dynamically in every call, e,g new AmazonProductList($seller_id, $marketplace_id, $key_id, $secret_key..) not using a config file.

@Peardian
Copy link
Collaborator

Peardian commented Aug 8, 2016

That sounds like something worth supporting. I don't know how long it will be before I can work on it, but look into adding this feature when I can. Thank you for bringing it up.

@ahmettok
Copy link

ahmettok commented Aug 8, 2016

I think it is a great feature to add, I also use the library for multiple users and I generate a new config file, every time I run the application, Queue the requests so that only 1 job is done.

@morphers82
Copy link

There are tons of apps out there that access MWS on behalf of their users, this is definately not an edge case, in fact i think accessing mws for just a single seller is probably more of an edge case than accessing using a Developer Accounts Key/SecretKey plus the user's merchantId and a MWSAuthToken that has been issued by Amazon to be used by the developer account to access the MWS API on behalf of the merchant. Thanks!

@stevechilds
Copy link

stevechilds commented Sep 26, 2016

I would echo this. I'm developing a integration using Symfony and its really not ideal to have to configure a php script within the vendors folder OR as I've done, create another PHP script in app/config and pass it into every constructor in the library when they're called.

The easiest solution would be to change it to accepting an array in the same parameter and doing a simple is_array() check, if it is, grab them, from there, if not use the existing code, then it doesn't introduce a BC break.

Edit: Actually is made worse by the fact the config param changes position with various constructor calls. I had planned to have a factory approach to generating them, this makes that harder as a now need to account for the fact that different constructors have them in different argument positions. :/

@Peardian
Copy link
Collaborator

My plan was to allow the first parameter, normally the store name, to be substituted for the array. After all, why would you need to give the store name if you're passing exactly the requirements you need? Unfortunately, I cannot work on it right now.

@stevechilds
Copy link

I've started to look into this, but oh hell! You've used the config file to set the store variable locally within each method it gets called in! :/ OMG, I can see why its a nightmare now. Sorry, but that's really, really awful design.

Ok, I've just searched and "include($this->config)" may only be in just under 30 places. What I would suggest (and I don't mind doing this, as long as the library does what we need it to do), is changing it so that the stores are read in and saved in the core class when its constructed, so that you don't need to include the config file each time you want to access them. You can pass them as an array in the store param as you suggest, these then get saved into the AmazonCore class.

Then wherever in the code include($this->config) is called, the method is changed to get the stores from the class property rather than the $store variable within the scope of the method.

The revised existing methods would still work with the current config file method as that file would be read in once in the AmazonCore constructor and saved into the object in the same way.

Thus accessing the data is the same irrespective of whether the config data is file based or array based.

Out of interest, why weren't the stores persisted into the Core object, as you do for some of the other settings?

@Peardian
Copy link
Collaborator

I won't defend my code on that point. After all, I wrote it 3-4 years ago as my first library.

I think that persisting the config data to the object would be a good idea, though I am not sure how exactly that could be done without changing the structure of the config file. After all, the config file contains several variables in addition to the store array, and all of that information needs to be passed to any created child objects. Those variables might need to be added to the store array anyway for the "config without file" idea to work. Perhaps that is why I designed it this way, so that only a single file path needs to be handed down.

Maybe there could be a configuration object? It could handle all of the loading/storing of config information and would simplify the passing of configuration to other classes. Just a thought that came to me while reading your message.

Regardless of how the store settings are saved, I think only the relevant store's array should be kept rather than all of them. I want to keep as little sensitive information stored in the object as necessary.

@stevechilds
Copy link

@Peardian I appreciate the code is old, I wrote some really bad code in the early days! We'll sort it though :)

I was thinking along the same lines and have created a configuration class/object which either takes an array or a file to keep backward compatibility. The core classes then refer to the config object rather than including the file all the time.

I've got it up and running with the simple test (getServerStatus) I had, now I just need to address the 221 test failures!

I've moved the logFile validation into the config class too, so I also solved the LogFile creation issue at the same time.

I'll add some new test cases too for the new functionality :)

I am trying to keep breaking changes to a minimum, so far there shouldn't be any! (fingers crossed)

@stevechilds
Copy link

Done :) All tests pass! I'll do a bit more testing, make sure I haven't missed anything and I'll commit the changes for you to review later today.

@stevechilds
Copy link

Ok, Pull request #103 added for your review @Peardian

@Fabr9193
Copy link

Hello,

I cloned this github today and I was wondering if there is some documentation about this new way of passing MWS somewhere on the repo.

Thanks in advance !

@stevechilds
Copy link

There should be a demo for using it? I added it in my fork before submitting the pull request. This is my fork: https://github.com/schildsCC/phpAmazonMWS

@Fabr9193
Copy link

@schildsCC Indeed there is I searched a bit more and I found your fork ;)

Thanks !

@Fabr9193
Copy link

Is it possible to include your repository in packagist ? Because I'm having the minimum-stablity issue when I try to install this repo with composer

@stevechilds
Copy link

I'm a bit busy today and I've never done that, but I'll have a look at it this evening. I don't really want to create another fork that ultimately I won't maintain as I'm about to leave this job where I needed to use the library, at my new job I won't be using it and thus, can't really guarantee maintaining it.

What I did with my symfony project was to copy it into the src folder and used it from there - I don't know if you could do something similar as a temporary measure (admittedly its not ideal).

There's loads of abandoned forks around, I just don't want to add to the pile!

Regards

Steve.


From: Fabr9193 [email protected]
Sent: 23 November 2016 10:10:42
To: CPIGroup/phpAmazonMWS
Cc: Steve Childs; Mention
Subject: Re: [CPIGroup/phpAmazonMWS] Pass MWS keys without config file (#97)

Is it possible to include your repository in packagist ? Because I'm having the minimum-stablity issue when I try to install this repo with composer

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//issues/97#issuecomment-262475186, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AVaTGBdIUgFSm4gliicvA8I2Y09Bqfsaks5rBBEigaJpZM4JeU2P.

@Fabr9193
Copy link

I understand 😄
I think i'll go for the temporary solution for now on.

Thanks

@Fabr9193
Copy link

@schildsCC When I try your example (getAmazonFeedStatusB) in the examples Folder it says that I can't reach the value of secretKey which is in [stores[storeName[secretKey]]]

Here is my code :
`
$amazonConfig = array(
'stores' =>
array('YourAmazonStore' =>
array(
'merchantId' => 'AAAAAAAAAAA',
'marketplaceId' => 'A13V1IB3VIYZZH',
'keyId' => 'AAAAAAAAAAAAAAAAAA',
'secretKey' => 'AAAAAAAAAAAAAAAAAAAAAAAAAA',
'serviceUrl' => '',
'MWSAuthToken' => 'amzn.******************',
)
),
'AMAZON_SERVICE_URL' => 'https://mws-eu.amazonservices.com', // eu store
'logpath' => DIR . './amazon_mws.log',
'logfunction' => '',
'muteLog' => false
);

    $anotherTest = new \AmazonMWSConfig();
    echo "Creation instance MWSConfig";
    $anotherTest->addStore('YourAmazonStore',$amazonConfig);
    dump($anotherTest->getStore('YourAmazonStore'));
    dump($this->syncFeed($anotherTest));
    exit();
}


public function syncFeed(\AmazonMWSConfig $configObject)

{
    $amz=new \AmazonFeedList($configObject->getConfigFor('YourAmazonStore'));
    $amz->setTimeLimits('- 24 hours'); //limit time frame for feeds to any updated since the given time
    $amz->setFeedStatuses(array("_SUBMITTED_", "_IN_PROGRESS_", "_DONE_")); //exclude cancelled feeds
    $amz->fetchFeedSubmissions(); //this is what actually sends the request
    return $amz->getFeedList();

}`

@stevechilds
Copy link

stevechilds commented Nov 23, 2016

@Fabr9193
Your code is incorrect... look at the function getAmazonFeedStatusB() in the example..

You need to pass the array through to the AmazonMWSConfig constructor and remove the addStore call, i.e.

$anotherTest = new \AmazonMWSConfig($amazonConfig);
echo "Creation instance MWSConfig";
//  $anotherTest->addStore('YourAmazonStore',$amazonConfig);
// ...
dump($this->syncFeed($anotherTest));

@Fabr9193
Copy link

Fabr9193 commented Nov 23, 2016

Indeed it now works ! Yeah I was just desperate I might have changed some of my code and forgot to put it right.

Thank you !

@buddyy93
Copy link

buddyy93 commented Aug 4, 2018

@stevechilds hi, i just tried your codes, but i'm getting this error.

A PHP Error was encountered

Severity: Notice

Message: Undefined index: code

Filename: classes/AmazonCore.php

Line Number: 605

Backtrace:

File: E:\MyServer\spapp\vendor\cpigroup\php-amazon-mws\includes\classes\AmazonCore.php
Line: 605
Function: _error_handler

can you help? thank you

@buddyy93
Copy link

buddyy93 commented Aug 4, 2018

sorry,there are my codes.

$amazonConfig = array(
'stores' =>
array('sales_planet' =>
array(
'merchantId' => 'HIDDEN',
'marketplaceId' => 'HIDDEN',
'keyId' => 'HIDDEN',
'secretKey' => 'HIDDEN',
'serviceUrl' => 'https://mws.amazonservices.com/',
'MWSAuthToken' => '',
)
),
'AMAZON_SERVICE_URL' => 'https://mws.amazonservices.com/',
'logpath' => DIR . './logs/amazon_mws.log',
'logfunction' => '',
'muteLog' => false
);
$configObject = new AmazonMWSConfig($amazonConfig);
$obj = new AmazonReportRequest($configObject->getConfigFor('sales_planet')); //store name matches the array key in the config file
$obj->setStore('sales_planet');
$obj->setReportType('GET_MERCHANT_LISTINGS_DATA');
$obj->setTimeLimits('2018-08-01', '2018-08-02');
$obj->requestReport(); //this is what actually sends the request
$requestreport = $obj->getResponse();
print_r($requestreport);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants