Traumflug Posted March 7, 2017 Posted March 7, 2017 By profession I'm more a server admin and embedded developer, so please bear with my ignorance. Nevertheless I currently have a PS 1.7 shop almost ready for production. What I miss in pretty much any shop software is a safe way to handle maintenance. A way to deal with upgrades, patches, code improvements, without interrupting services to the customer. It looks much like every merchant just tweaks the running shop and hopes the best s/he makes no mistake. No strategy other than quickly reverting wrongdoings or, if things go really wrong, take the shop down to recover from a backup. 30B's stated goal is even to allow code editing from backoffice, surgery on the open, beating heart! As server admin I've learned that one shouldn't do such things unless one doesn't care about customer experience or enjoys to live on the dangerous side. The safe way is to do maintenance offline. On the public web server in a separate installation or on a local installation on the developer's machine. Then to test, test, test. Only after tests were satisfying, development code is synchronized with the public server. This is safe and comes with a minimum of downtime, which means a minimum of customer disruption. However, and this is the reason of this post, this requires a way to clone or synchronize a shop. Rsync shop-here to shop-there with the confidence that the copy works the same way as the origin. AFAIK, PrestaShop doesn't allow such a strategy. Server name is stored in the database, so one has to tweak that manually after an upload. Even installation location on the server is stored somewhere, one can't move a shop from one folder to another without breaking it. Wether a module is present or not isn't determined by looking at files, but by looking at some DB entry, making things inconsistent when one just removes or adds module files. Are there ThirtyBees plans to improve this situation? For PS I made a couple of patches which resolve most issues for monoshops (multishops are more complex). Are such improvements welcome? What do others think?
Traumflug Posted March 7, 2017 Author Posted March 7, 2017 Of course. Note that these patches are done for PS 1.7. They should be sufficient for discussing the strategy, I'll happily port them to TB, then. Patch 1 is about the shop domain. PS uses the PSSHOPDOMAIN configuration variable for this. This variable is used in dozens of places, so I decided to override the value read from the database, instead. Nice side effect: previous behavior is kept, new behavior is applied if that field is set to 'automatic' in back office, only. 014888992878330001-CO-Allow-automatic-PSSHOPDOMAIN.patch Patch 2 does the same for installation location. 014888996240770001-CO-Allow-automatic-for-Base-URI-too.patch Patch 3 also tweaks a configuration variable, this time PSSHOPENABLE. Instead of storing that value in the database, it's stored by presence of a file. This way one can easily set a store into maintenance mode by uploading a file by FTP (name matters), then do the synchronization, then remove the file. Scripts are very good at doing such sequences : - ) 014888999404540001-CO-store-PSSHOPENABLED-not-in-the-database-but-by-.patch While patch 2 appears to work so far I could think of a better strategy in this case. Code location has no interference with multishop capabilities, so the IMHO better way would be to get rid of that database entry entirely in favor of always automatic detection. Not done this way so far, because I have to keep code changes small for not running into conflicts on the next PS upgrade (or migration to TB). P.S.: glad to see one can upload patches in this forum!
Traumflug Posted March 7, 2017 Author Posted March 7, 2017 Glad to see you like it. TBH, the more I think about the issue, the more I think the right way would be to get rid of all this code in classes/shop/Shop.php and classes/shop/ShopURL.php which tries to figure the "right" URL and also tries to redirect to that. A monoshop shouldn't care about the domain it's hosted on. If a merchant wants to redirect something, e.g. http:// -> https:// or typo-domain -> real domain, .htaccess is the right place to do so. A multishop should just read the domain, not try to "fix" it. If a domain happens to be not one of the multishop domains, that's a configuration error, nothing which should be validated at runtime on every single page request. Removing all that code would also remove a number of database queries, as far as I can see. I think I'll play a bit with this idea.
Traumflug Posted March 8, 2017 Author Posted March 8, 2017 Let me put a status update here. I managed to get a development shop running from a clone of the Git repo. There I applied this 'automatic' branch and added a couple of additional things: An attempt to not read shop domains from the database in classes/shop/Shop.php at all. Which means to remove all this redirection code there, speeding up page loads. Doing redirection via .htaccess. Writing .htaccess in backoffice adds a few lines to do this redirection. All three parts work nicely for monoshops. When trying to review the first commit for multishops I enabled multishops. After doing so, frontoffice was broken (infinite redirects), so this first commit apparently needs rework. Then I tried to add a second shop. This messed up the database somehow. Whatever I try to access, backoffice or frontoffice, results in "Internal Server Error". Disabling multishop via phpMyAdmin didn't improve this. Next step here is to drop that shop for starting over. Code is here: https://github.com/Traumflug/ThirtyBees/commits/ignoredomain P.S.: Generally, all this redirection stuff is very complex. I found at least 8 code locations dealing with it. Looks like there are at least four levels: domain "fixing" in Shop.php, something similar in FrontController.php, some widespread mechanism to redirect depending on content and an approach distinct from all this for switching between http and https. This led me to the conclusion that not storing the domain somehow would be a way to intrusive change. New plan after fixing the commits described above is to just move the configuration table into an ordinary file, making it accessible for upload scripts.
crosscut01 Posted March 9, 2017 Posted March 9, 2017 Very nice work y'all, this will be nice to have.
Traumflug Posted March 12, 2017 Author Posted March 12, 2017 Next update. Progress is slow, but I learn a lot along the way :-) Today I tried to put a shim between ShopUrlCore and ObjectModel. The idea is to override all database accesses with accesses to the array in question. This shim is class ObjectFileModel. Code is on branch 'objectfilemodel': https://github.com/Traumflug/ThirtyBees/tree/objectfilemodel This mostly works, but one way only. The array gets updated after e.g. adding a new URL to a shop. But the page load following that falls back to reading the DB. Because page rendering uses AdminController, which accesses the database directly, ignoring ObjectModel. Looks like I have to try to put a shim between AdminShopUrlController and AdminController tomorrow, too. Another idea is to connect AdminShopUrlController to ShopUrlCore directly, but then I'd loose all the validation, field forming and relational (multishop, multilanguage) code.
Traumflug Posted March 18, 2017 Author Posted March 18, 2017 Saturday night update. The shim between ShopUrlCore and ObjectModel is now well established as 'ObjectFileModel'. Things start to work, I think editing multishop URLs (backoffice -> Advanced parameters -> Multishop) is complete already. This shim between AdminShopUrlController and AdminController was tried, but dropped. Instead I taught ObjectFileModel to not only write, but also read entries. Typical for the Model-View-Controller concept, but PrestaShop apparently prefers to read from the DB directly (somewhat understandable, one can do some data shaping, like sorting, inside DB queries). Code is on branch ‘objectfilemodel’: https://github.com/Traumflug/ThirtyBees/tree/objectfilemodel , rebased to latest 'master'. Branch length currently 12 commits. Things get clearer, commits smaller, which is usually a good sign. Currently hunting down all usages of the 'shop_url' table and replacing that with the file based storage. :-)
Traumflug Posted March 25, 2017 Author Posted March 25, 2017 Saturday night update. Reviewed each commit again and I think the existing ones are usable now. As I implemented synchronization between file based and database based storage (will go away later), every single commit keeps a fully working front- and backoffice. Still working on eliminating shop_url database table usage, step by step. Code is freshly pushed on https://github.com/Traumflug/ThirtyBees/tree/objectfilemodel , 19 commits by now. Guess is, another 20 are required to complete this task.
Traumflug Posted March 25, 2017 Author Posted March 25, 2017 Phinx looks like a good option if the task were to get rid of the database entirely. For handling just one 'table' it's overfeatured. Also, the "engine" is already completed. It's just an ordinary PHP array, and writing that is as simple as PHP file_put_contents($this->def['path'], "<?php\n\n". 'global $shopUrlConfig;'."\n\n". '$shopUrlConfig = '.var_export($shopUrlConfig, true).';'."\n"); Reading it is as simple as ... well, it's already there, in a global variable. For better abstraction there's ShopUrl::get() Most of the work is to replace queries also accessing other tables or doing some data shaping. Queries with WHERE, JOIN LEFT, CONCAT or ORDER inside (pretty much any query). This has to be split into a DB access for only the other table(s), then mixing and reshaping both results to get what's required. Phinx won't allow to query both, file based and DB based storage, with a single query, as far as I can see, so no advantage for this part.
Traumflug Posted March 27, 2017 Author Posted March 27, 2017 Good news: I removed the shop_url table here and the shop continues to work :-) I hope I catched everything. Stuff like this: PHP foreach (Shop::getAssoTables() as $tableName => $row) { // ... $res = Db::getInstance()->getRow('SELECT * FROM `'._DB_PREFIX_.$tableName.'` WHERE `'.$id.'` = '.(int) $oldId); is hard to catch, because not grep-able. For now I simply hope this is never called with $tableName = 'shop_url'. Next step is to adjust the installer. Then to get rid of PSSHOPDOMAIN, PSSHOPDOMAIN_SSL, etc.
Traumflug Posted March 29, 2017 Author Posted March 29, 2017 The part replacing the shop_url DB table by a file is done, see https://github.com/thirtybees/ThirtyBees/pull/162 To achieve fully scriptable shop synchronization, configuration variables PSSHOPDOMAIN, PSSHOPDOMAIN_SSL and a few others have to go away, too. That's a separate task. Code for backwards compatibility will be provided, of course, so a Configuration::get('PS_SHOP_DOMAIN'); will still succeed, even if there's no longer such a database entry.
MockoB Posted March 30, 2017 Posted March 30, 2017 @Traumflug I follow with interest your topic although I rather read Chinese :) I don't know why but I feel excited about your achievement :) Congrats!
kpodemski Posted March 31, 2017 Posted March 31, 2017 @Traumflug i'm wondering, is not having hardcoded "shop_url" really helps you a lot? because for me it's bs, can you give me real life example of your workflow, for the store upgrade? what is easier before and after your changes? i'm trying to understand your point, but for me hardcoded shop_url, is one of the smallest problems, in terms of PrestaShop sync between dev and production version
Traumflug Posted March 31, 2017 Author Posted March 31, 2017 "bs", like "beautiful surprise"? A real life example? sh cd /var/www/html/ cp -rp my-shop-test my-shop-live After that, 'my-shop-live' won't work without digging into backoffice and fixing things there. Which in turn disables 'my-shop-test'. Another example: - Go to http://localhost/ and install ThirtyBees - Go to http://127.0.0.1/ The latter won't work either, it'll redirect to localhost. Even if it's a small problem, it should be fixed. However, I'm not aware of any other problems preventing shop synchronization. And to some extent I wonder why you lament here and on Github instead of showing us on how to synchronize a TB/PS-shop. If the strategy described in the opening post here is achievable so easily, post the solution!
MockoB Posted March 31, 2017 Posted March 31, 2017 I don't claim to understand what he actually doing but the way I get it is: I could make a copy of my live site on my local machine and vice versa at any time. I believe the hardcoded shop URL is barrier for ignorant merchants like me. You could search prestashop forum and you will find countless topics concerning that. But I am not a developer and that's the way I understand it!
kpodemski Posted March 31, 2017 Posted March 31, 2017 @MockoB said in Safe maintenance strategy, shop cloning: I don't claim to understand what he actually doing but the way I get it is: I could make a copy of my live site on my local machine and vice versa at any time. I believe the hardcoded shop URL is barrier for ignorant merchants like me. You could search prestashop forum and you will find countless topics concerning that. But I am not a developer and that's the way I understand it! yeah, i think that is some example which i'm willing to understand, but the word "synchronize" is completely out of topic here, because "shop_url" is the easiest thing to do, while working on developer, and production store, at the same time, really @Traumflug said in Safe maintenance strategy, shop cloning: "bs", like "beautiful surprise"? A real life example? sh cd /var/www/html/ cp -rp my-shop-test my-shop-live After that, 'my-shop-live' won't work without digging into backoffice and fixing things there. Which in turn disables 'my-shop-test'. Another example: - Go to http://localhost/ and install ThirtyBees - Go to http://127.0.0.1/ The latter won't work either, it'll redirect to localhost. Even if it's a small problem, it should be fixed. However, I'm not aware of any other problems preventing shop synchronization. And to some extent I wonder why you lament here and on Github instead of showing us on how to synchronize a TB/PS-shop. If the strategy described in the opening post here is achievable so easily, post the solution! Well, lament? Maybe because it's wasting time and energy, both you, and Michael's who needs to check this feature. Ok, you can copy production site to localhost, and what later? Hm? What about differences between two databases? Or, you want to work with remote production database, on localhost? I guess not. I wish TB all the best, and when i see wasting time on such a feature... it's not looking good :)
kpodemski Posted March 31, 2017 Posted March 31, 2017 All i said... if this is really just for shop cloning, and you just make so much changes in core, just to save 1-2 minutes of doing changes on duplicated database in some SQL client... i don't know if that makes sense
Traumflug Posted March 31, 2017 Author Posted March 31, 2017 No problem if its just time. It's not your time and for me this time is easily compensated by reduced shop downtime and an upload script not forgetting to fix the database. Once this works I can simply ./upload.sh and forget about all the extra steps. Each time I do an upgrade or change something on the theme design. What about differences between two databases? They shouldn't matter. Functionality and design should be independent from content.
roband7 Posted March 31, 2017 Posted March 31, 2017 @kpodemski It´s not 'thirty bees' wasting time. This is a classical case of a developer scratching his own itch. This is covered in classic open source documents like Dave Thomas "The Pragmatic Programmers" or Eric S. Raymond "The Cathedral and the Bazaar" In my opinion it´s crucial that developers scratch their own itch, and that thirty bees accept such contributions. It´s what will grow the community and make this much better than Prestashop in the long run.
MockoB Posted March 31, 2017 Posted March 31, 2017 And @kpodemski if it was that easy there won't be so much topics like "my shop is dead after update" etc.
kpodemski Posted March 31, 2017 Posted March 31, 2017 @MockoB i don't belive that someone who is doing update of the production, will instantly go to local env because of this change, you know :) well, keep sratching guys, good luck :P
Traumflug Posted April 1, 2017 Author Posted April 1, 2017 The set of commits moving table 'shop_url' is to a file is already on master, thank you to Michael for accepting it. Next pull request is about removing configuration variables PSSHOPDOMAIN and PSSHOPDOMAINSSL. My impression is, these two were kind of deprecated before already, even the backoffice page for setting them used the 'shopurl' table already. Pull request is here: https://github.com/thirtybees/ThirtyBees/pull/164 To those thinking now "OMG! He breaks virtually all modules!": There's backwards compatibility code. A Configuration::get('PSSHOPDOMAIN') still succeeds properly (and returns the domain of the first main URL). Just querying the database directly, something code shouldn't do anyways, breaks. Replacement is straightforward, see this commit message: https://github.com/thirtybees/ThirtyBees/commit/0614b131da05623c60ba1fe2f74dd23fa53cd626
alwayspaws Posted April 2, 2017 Posted April 2, 2017 @MockoB said in Safe maintenance strategy, shop cloning: @Traumflug I follow with interest your topic although I rather read Chinese :) I don't know why but I feel excited about your achievement :) Congrats! LOL! Same for me! It's wonderful to have a server admin / embedded developer devote so much time to thirty bees!
Traumflug Posted April 3, 2017 Author Posted April 3, 2017 Hmm. Postponing this for 1.1.0 is a good idea. TBH, I was a bit surprised you accepted that pull request with no less than 26 commits so quickly and for a bugfix release. If you can describe a bit what "unstable" means I'll work on that, of course. Now I can also search for a way to eliminate this global variable while keeping cache-ability.
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now