Laravel Code Review: Multi-Tenancy, Events and Queues

hello guys today we will have another code review on this channel and this time not junior code review more experienced developer who created multi-tenancy with multi-database with sas on top so a project like this where business owner could sign up create their own listing projects which would land on sub domain on a different database and they would be able to log in as that project admin and manage their listings interesting part that admin panel for that is based on our quick admin panel which was a pleasant surprise and also that developer used one of our demo repositories which is laravel and wire sas generator we had a sas generator in the past but it didn't pay off as a product so we kind of shut it down it still works but we don't advertise it anymore but there are a few repositories online for free and i will link them in the description of this video and let's see what advice i can give for multi-tenancy multi database so first installation of the project i emphasize more and more during these code reviews that if you give that project for someone else to install even for yourself in the future or on a new server that instruction readme should be clear and working and when i try to install that project according to those instructions i land and i get unauthorized and as i know that this project is multi-database and multi-tenancy probably i didn't configure some domain so let's take a look in the routes web php i see home page as website controller let's open website controller and it is simple view so there's no unauthorized middleware or something here where else can we find middleware if it's not in the routes then it's one layer above which is route service provider which registers all the route files and in here we have in the map map api routes map web routes and map tenant rounds so the third one is a custom thing these two come from laravel and the third one we will get to that so let's click map web routes which is exactly what we need and here i see middleware check master domain so probably that one is blocking us from entering how to find that middleware we go to app http kernel file and we check for check master domain which is this one okay what's inside okay we check if the host is not config oh config so i had to enter that tenant master domain let's go to config so config it should be a file called config tenant and here we have master domain which is by default sas based test and in my case i've configured my laravel valet to have sas.test so that doesn't match and it shows unauthorized so my first advice if there is something to prepare to launch the project that should be in either readme or in.env.example or in both so that app domain should be in env so if i open that dot env and let's open sas test here in the browser and let's see if it works and it worked so advice for any of you developers and to the author you should go to dot env.example and put all the variables here that other developers need but just with empty values and of course update the readme that you need to configure that domain okay we're in on a home page next let's try to sign up to register and this should probably be called sign up so log in and sign up from what i remember in english language so we fill in with fake filler register and we're into the dashboard what do i see on the dashboard i can create a project and as it will turn out in the future it will create that database and subdomain for me so if you add a project for example let's call it a test and then subdomain should be test so the final result subdomain will be test.sas.test or actually let's name it something differently so first sas test right and what happens before i click anything what happens when i do create in our routes web we have projects controller here somewhere here projects controller and if we open that projects controller store this is what happens let's close the sidebar so we create the project with db name and all the other parameters no and then there is an event being fired project created that event is called by a few listeners so if we go to event service provider this is from default laravel and these two are custom so on project created event there is a create project database listener if we search for create project database listener what it does is create the database and there's a separate class database manager let's take a look inside it does just the creating the database part and then if it fails it throws exception and then it throws another event which is called by another listener so in the event service provider run tenant database migration is this which calls tenants migrate so artisan command tenants migrate i hope you're following but we will analyze that in a bit detail for now just overview so app console commands talent migration is an artisan command which does the migration for some tenant if it is a specific tenant we exact command on the project for a specific tenant otherwise we are running the exact command for all of the projects and that exact command is connecting to the database set connection and artisan call migrations from the path and this is interesting so there are database migrations for main projects so main project is here but then there's database migrations tenant and there are two separate migrations for admins and admins will be a separate model in app models tenant admin so it uses a guard separate off guard to do that well anyway it launches those migrations and launches admin table seeder which is creating an admin with the same credentials as the main user let's try it out now so it should create a database db underscore first let's see if it works it works it shows another warning of maximum amount of projects that's related to stripe and that doesn't really matter let's see the database in our main database we have project with subdomain first and database name db first and then it actually created a separate new database on my local machine with admins migrations so everything that was in those listeners it seems that it has worked but let's take a look a bit deeper as i understand now i can launch first dot sas.test subdomain and it would work let's try it out we launch first.sas.test and it's still unauthorized why so it works this loads laravel and my laravel overlay should recognize the subdomains and attach to the same project but it's unauthorized why because in the route service provider route service provider we get back to map web routes there is a domain being attached to map web routes and then there's domain bn attached to map tenant routes like this and the error here that app domain is lowercase so it's case sensitive if you specify app domain lowercase it's not the same as you specify in config tenant app domain in uppercase so one way to fix that should be uppercase here so app domain here and app domain in the routes web here and that should work in a minute i will show you what that means but it worked but even a better solution is not to use env directly here instead use config tenant master domain so config tenant master domain like this and same here for the tenant so subdomain that's cool but instead of env app domain should be this and let's refresh if we didn't break anything it still works so that's the main advice is to use config values first and then inside of that config we solve the values by pointing to the env or providing default values so now we loaded the urls from our map tenant routes and there's a separate tenant middleware which is different kernel php tenant is switch tenant database and it search for the project with that database if it's not found 404 otherwise set connection right and there's a separate routes tenant php let's open it up route tenant php and there's a separate set of routes specifically for that tenant subdomains and this is where i get that default connection from i think it's a security issue or probably it's just for testing purposes the project isn't done so that will go away at some point all will redirect to login so if we launch login slash login we have a login form it looks the same as in the main login but actually it's a different controller so admin login controller in tenant os so it's a different set of controllers so app http controllers tenant but we can log in with the same user so same user and the password is the same and we're in to the actual dashboard of tenant project so i made it work now let's comment what i could improve or what i would advise to improve so when i create the project i create a project with subdomain and then it creates the database it's a really huge operation and i would double check the validation so for example what do we have in that store project request we have name required anything about subdomain so there are no rules what would be the sub domain i would be very strict with the names because it creates the actual database on your server so this should be not only required unique but then also some alpha num or even more strict rules on like only letters or something like that because at the moment i can create a project with another the same subdomain and i don't even predict what would happen would it override the database would it throw an error or what would happen don't know so more validation on these operations next the event of project created and it's cool that it's using event and not just in line in the controller but i would challenge the idea that there are two different events and two different listeners so if the project is created then we create the database here why don't we create the migrations here in line do we actually need another event that would be caught by another listener to run migrations i'm not sure maybe there's a reason for that and maybe i'm missing something but i wouldn't create probably another listener and another event and i would do the migrations here because if database creation fails it would throw an exception and wouldn't even go to this part so that's one thing and another thing with events i'm not entirely sure that it's the best pattern to use i would probably create a job for that and i would put that job into the queue in this case the database is pretty small with only a few database tables so it launches quickly but if that database grows that tenant database the structure grows or some admin or some more stuff so what i would do is instead of having event project created do something like php artisan make job create project or create project database and then fire dispatch or this dispatch create project job i don't remember the syntax actually it's actually check the laravel documentation laravel jobs dispatching jobs podcast create dispatch okay so it's a bit different syntax so create project dispatch create project dispatch like this and then that job create project implements shoot queue and that's the main thing it should be put in the queue and then everything here everything that is now in listener should be actually a part of the job that's not the only thing to do that that's my personal opinion and that events and listeners pattern is one way of doing that but from what i understand it's not queueable so it's happening in the sync whereas in fact it should have been put in the queue because it's a more complex operation and may take longer and also i'm not sure it should have been an artisan command so inside of that listener run tenon database migration is artisan call for talents my grade i would probably put that into the same queue to migrate the tenant database maybe the reason was that that developer launches that as artisan command as well so not only from the web so maybe it makes sense to have it an artisan command but i would challenge that and maybe would put that in the same job and in the queue so as i'm talking i realize this is another proof that there are very many ways to structure the project so if you have some background operations like creating database or something like that there are many ways how you can structure that with events and listeners maybe with observers maybe with jobs and queues maybe with some custom actions like app actions folder whatever so you shouldn't take my advice for granted i'm telling that to that developer to the author or to any one of you i'm just sharing my opinion what i would maybe do but i haven't actually done it and maybe in the process of doing that i would realize that that way of doing that is actually more correct than mine and also you may realize that this way is more correct when the project grows so when you have thousands of tenants maybe you will realize that you have some performance issues or the code is not that maintainable or readable and then maybe you will change the pattern so as they say there is no one way to skin the cat so here i would like to end that review because the topic was multi-tenancy in multi-database i've reviewed this part the repository is not public and i don't have any permission to publish that because in other code reviews some of you asked in the comments to publish the repository i don't have permissions to do that it's not my code but if you have any questions to the author shoot in the comments i'm pretty sure he will be reading the comments and maybe we'll reply on maybe we'll give you access to the database don't know but that's it for this review if you want to support me on my mission of daily videos and these code reviews check out one of the three products that you can see on the screen quick admin panel which was actually used to generate part of that admin panel in this project than my set of courses which is 16 courses at the moment and i'm shooting new ones in 2021 and also my live market set of components see you guys in other videos

Channel: Laravel Daily

Views: 18,367

Keywords:

Id: 8ot9IiGaqhk

Length: 14min 40sec (880 seconds)

Published: Fri Mar 19 2021