Skip to content
This repository has been archived by the owner on Jan 6, 2023. It is now read-only.

[WIP] PostgreSQL support #1319

Open
wants to merge 62 commits into
base: master
Choose a base branch
from
Open

[WIP] PostgreSQL support #1319

wants to merge 62 commits into from

Conversation

Toilal
Copy link

@Toilal Toilal commented Sep 26, 2019

This is a work in progress for PostgreSQL support (#369)

I have merged and cleanup pgsql branch, squashing everything in a single first commit and on top of current develop.

I'll try to rebase this branch on develop as much as possible and provide everything required to support various database vendor.

TODO:

  • Make installer works with both MySQL and PostgreSQL (requires app from directus/app#2097 branch to allow database type selection).
  • Make migrations work with both MySQL and PostgreSQL.
  • Use composer-patches to apply zend-db patches on latest version
  • Support collection creation on both MySQL and PostgreSQL.
  • Support collection deletion on both MySQL and PostgreSQL.
  • Support adding fields on both MySQL and PostgreSQL.
  • Support removing fields on both MySQL and PostgreSQL.
  • Extract all MySQL specifics into SchemaInterface implementation (MySQLSchema)
  • Provide automated tests for both MySQL and PostgreSQL setups

binal-7span and others added 30 commits August 8, 2019 16:37
* Parent + Nested validation changes (directus#1138)

* Add migration schema for 2FA Secret user field

* Add 2fa_secret field to FieldsSeeder

* Create Missing 2FA Password Exception

* Add googleauthenticator dependency

* Add getter for User's 2FA secret

* Check for otp param in login request, and login with it

* Add enforce_2fa parameter to directus_settings

* Create Utils endpoint and service method to generate 2fa secret

* Add enforce_2fa field to roles

* Add enforce_2fa field to FieldsSeeder

* Change Missing2FAPasswordException error code to 111

* Change 2FA Library

* Change 2fa_secret interface in FieldsSeeder

* Created exception for invalid otp

* Changed findUserWithCredentials to through an InvalidOTPException on otp check

* Created new exception if 2fa is enforced but not enabled by user

* Added function to check if 2fa is enforced for a user

* Check in AuthenticationMiddleware whether 2fa is enforced and enabled for user

* Add optional needs2FA field to auth token and on token refresh

* Catch error if enforce_2fa column doesn't exist
Fixes crash when has2FAEnforced is called on a DB that hasn't been migrated

* Use relative positions for target path array to check user edit

* Fix unset on payload_arr instead of payload

* Change 2FA activation on login to use activate2FA endpoint

* Update ItemsService.php
There are cases when $type is not a string but an object that inherits from ObjectType.
In that situation array_key_exists failing because it should get only integers or strings 
as a first parameter. So in order to avoid that the 'name' property of the object is used 
as a key.
Adds in detection and parsing for youtu.be shorthand URLs.
* added file meta data for audio/video

* updates as per PR feedback
* Fix for smtp send mail issue directus#1205

Missing additional config settings
directus#1205

* Update Schema.php
* Change namespace for PHPUnit.

* Removing old tests.
* Fix directus#1238[PDF support for Embedded URL]

* Add array support

* Update comment

* Update Files.php
directus#1257)

* Fix directus#1255 [Support batch create/update/delete for user]

* Add exception
@Toilal
Copy link
Author

Toilal commented Sep 26, 2019

How about migrating the fork of zend-db dependency to composer-patches ? The fork seems only useful to support dashes (-) in parameters now. It seems it included other fixes, but they have been fixed upstream now. (see merge at directus/zend-db#1)

Diff between released version and this fork is really small (after merge)

This pull request is now using zend-db patches from this branch

@benhaynes
Copy link
Member

Hey @Toilal@BJGajjar will be writing a summary of the differences in our Zend fork, and why we need/needed it. Our previous dev had created the fork and Binal inherited that code, so it might take some time for her to audit things.

@Toilal
Copy link
Author

Toilal commented Sep 27, 2019

Thanks @benhaynes, It seems some changes have been fixed upstream, and remaining changes from the fork consist only of this patch file : https://github.com/directus/api/blob/77acbd0fd0d1d19dc22d6e6f825854dc11ba10c7/patches/zend-db/0001-Add-support-for-more-column-names.patch

@benhaynes
Copy link
Member

Interesting, thank you for the research! That will help @BJGajjar get this documented...

Good to know they've resolved the other issues upstream, though I wonder why we have a difference in column naming support. The main one seems to be dashes in column names... which is fine in SQL.

@webdeb
Copy link

webdeb commented Dec 9, 2019

Guys, whats the plan here, it would be really sad if that issue stays as WIP for some other months. How can we help?

@benhaynes
Copy link
Member

Hey @webdeb — there are a few facets to this... that all deal with how difficult it is to build maintain software this complex with zero budget.

  1. There is a lot of work required to fully merge upstream, review, test, update/fix, and merge this PR. This step can be done mostly by the community, which helps a lot. With v8 and Directus Cloud in our immediate pipeline, I don't think we (Core Team) will be able to perform this for at least a few months... so it's up to the community.

  2. Maintain the codebase for both SQL vendors moving forward. This is the hard one, because contributors have a tendency to disappear, and our small team is left maintaining a far more complex codebase. Or, more specifically, we've instantly doubled the testing that needs to be done for every fix/feature/release. That's the main problem, Postgres would be awesome to add, but we (Core team) don't really need it for our internal purposes, but it will really slow down all our other Directus development.

I'd be much more active in getting this integrated if it were to become truly database agnostic (including NoSQL, etc), but our current API codebase has too many exceptions built-in to easily do that. We're in the process of aligning on our next big API refactor (PHP and/or Node) which will be the time to do that properly. But until then, I worry that this will be a lot of work and cause a lot of deep bugs that we then need to officially debug to maintain support.

I know those who have worked on this PR and want Postgres will be bummed to hear this... but just remember that this platform codebase is OSS, and therefore beholden to those subsidizing its crazy costs and/or spending years keeping its momentum.

@webdeb
Copy link

webdeb commented Dec 9, 2019

@benhaynes Thank you very much for the time you took to explain it to us. I understand the problems you guys are facing, and for sure, if you don't need postgres for your internal purposes, better to not trying it to include it at all. It's better when the system does a good job for mysql and don't try to overcomplicate it. Then its actually better to rewrite it completely with an abstraction in mind from the beginning.

Btw. did I understand correctly, that you are planning a techshift to node? There is nothing wrong with that, just interested if you maybe considered also something other, like Elixir for example?

@benhaynes
Copy link
Member

Glad you understand! Hopefully our core resources will increase over the coming months/year since we're launching the Directus Cloud pay service to offset costs (starting this week!). Then enhancements like this are more feasible!

Yes, we're in the Discovery and Strategy phase of a Directus API refactor. We've reached out for input from the community over our Slack/Twitter, and have a GitHub ticket for consolidating ideas, etc. It would be difficult for us to leave PHP since we have millions of users on that stack, but we're looking at modernizing that codebase with Laravel and/or adding a Node.js port (with feature parity).

Obviously maintaining two API ports has similar resource implications, but there are more benefits to be seen here.. including full database abstraction and zero-lift installations. Open to learning about and discussing other options too, like Elixir... which I'm not familiar with. 👍

@rijkvanzanten rijkvanzanten reopened this Dec 12, 2019
@rijkvanzanten rijkvanzanten changed the base branch from develop to master December 12, 2019 20:04
@ddavidebor
Copy link

@benhaynes CMS companies that switch from PHP to NodeJS for bakckend have a history of failing, I've seen quite a few since I have been waiting for two years for Directus and similar CMS to get to the point where i need them for a big project. (Directus and Frappe being the only two good ones, with different strength and weaknesses).

There are reasons for this. NodeJS has not a stable enviroment to support a long-term project that needs stability and reliability in its database API, as most frameworks are fragmented, new, have small (competent) collaborators base and tend to die spontaneously after a couple of years.

I dislike PHP very much, but Lavarel database support covers:

  • MySQL
  • PostgreSQL
  • SQLite
  • SQL Server

This cover, respectively:

  • easy installations in web environments
  • more serious installations (cloud, business stuff, multi-tenant, lots of data...)
  • super-easy demo project
  • Big dumb companies who like to spend a lot of money and companies with weird integrations requirements with legacy dumb application

That's were the big $$$ are...

Only application case nodejs allows you to cover more easily than PHP are either really big (nosql distributed) or really fast (event-based databases) applications, neither of which has ever been needed for this kind of product.

it is very important to analyze the history and the people/companies of database frameworks very carefully, not to be blinded by the shiny code and futuristic paradigms.

my 2 cents, I'd hate to only be able to use Frappe (that is incredibly powerful, but really messy)

@benhaynes
Copy link
Member

Thanks @ddavidebor, some really helpful insight! @WoLfulus just started working on the Laravel port for our API... so hopefully we're on the right track!

@ddavidebor
Copy link

@benhaynes I'd say you are :-)

@shaunc
Copy link

shaunc commented Mar 26, 2020

I understand from this thread that (1) postgres isn't coming soon, and (2) you have decided against porting to node/typescript?

Too bad -- you do use typescript on the frontend. Vs @ddavidebor's point, nodejs is noisy because its so easy to use and publish a package, but you don't need "most frameworks" ... knex or sequelize are probably fine, for instance. We are a typescript/golang/postgres shop. We would love to use Directus, and maybe could contribute to something if we didn't have to learn PHP... but it remains just out of reach....

@benhaynes
Copy link
Member

Postgres is in work now in the new api-next refactor repo (for v10). This will port the API to Laravel, and add full test coverage and better database abstraction (including SQLite, MS-SQL, etc). Once the new API port is completely stable, we'll be breaking ground on a node.js port. Both would have feature parity and use the same App codebase and maintained in parallel.

Hope this helps give insight into where things are heading!

@rijkvanzanten
Copy link
Member

And that'll make sure the project will align 2/3rds with the typescript/golang/postgres shop stack 😉

@shaunc
Copy link

shaunc commented Mar 26, 2020

Wonderful ... any idea of the schedule? For our project, we want to be in beta in June and production in Sept. Given best the alternative (so far that we've found) is "roll your own" we'd be happy to see if we could make Directus work for us if Directus is reasonably on track to provide at least postgres and hopefully nodejs (so we can bang on it ourselves if necessary) at least a month before those dates. Getting up even a hacked dev version as soon as possible would also be important.

BTW "Headless CMS" seems to be quite unspecific vs what you are actually doing: more like "headless and tailless CMS" -- "thorax only". :) or "CMS as middleware". IMHO broad DB support is a very logical fit for what you do. We support a financial data service, with a large legacy db, that also publishes news & analysis on the basis of the data, and are looking for other clients with similar models. We can't use a CMS that has a bespoke data model, or at least not without having to maintain lots of glue bridging its internals to our clients' internals.

@benhaynes
Copy link
Member

Hey @shaunc — first off, I love the "tail-less" term... we had all sorts of weird descriptors to describe our unique approach (in the investor deck I've bene creating). Thanks for this awesome term! 😄

The rough timeline for the API port is 2 months to alpha/beta, and then hopefully no more than a month or so to be production-ready. Hope that helps as a rough guide!

@shaunc
Copy link

shaunc commented Mar 26, 2020

@benhaynes glad you like it (tail-less)! :) any pointers on what I should do to try to spin up a dev version? I notice in the docker image you don't have any config for DIRECTUS_DATABASE_TYPE ... should I try to run api-next or is that still not a reality (even for dev)?

@benhaynes
Copy link
Member

Hmm, that's beyond my skillset and a question for the actual devs: @directus/team 🔔

@guilhermeaiolfi
Copy link

Hi,

I have just discovered directus and after reading some comments about refactoring code like this one, I am not sure what version should I start testing and evaluating. I would like to use postgres if available, but since it is a new project, I am able to use MySQL if that's the best option right now. It's going to be a long term project, so I don't think some stability issues initially is blocking me from choosing postgres. What would be your recomendation? Is this somehow usable yet?

@benhaynes mentioned v10, but directus' medium page is "introducing" version 7, the php sdk (which the homepage redirects to) is talking about v6 (legacy). I am kind of confused how put everything together.

@rijkvanzanten
Copy link
Member

@guilhermeaiolfi I recommend you check / try out https://github.com/directus/next/

@guilhermeaiolfi
Copy link

@guilhermeaiolfi I recommend you check / try out https://github.com/directus/next/

What if I need to stay in the PHP land?

@benhaynes
Copy link
Member

@guilhermeaiolfi — then you can check out the Laravel port here: https://github.com/directus/api-next

Though this is a bit behind since it needs to follow the Node.js API Spec...

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

Successfully merging this pull request may close these issues.