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

Wrap instance reporting for database schema in an API #30877

Closed
miaulalala opened this issue Jan 27, 2022 · 1 comment · Fixed by #40423
Closed

Wrap instance reporting for database schema in an API #30877

miaulalala opened this issue Jan 27, 2022 · 1 comment · Fixed by #40423
Labels
1. to develop Accepted and waiting to be taken care of enhancement technical debt

Comments

@miaulalala
Copy link
Contributor

Problem

Currently, we're checking for the Database Type of an instance something like this:

$schema->getDatabasePlatform() instanceof PostgreSQL94Platform

If we ever change the underlying structure, these checks might break.

Proposed solution

Add a new API method for ISchemaWrapper that will return the database type as a string and a few constants that map to the supported databases to compare the Database string too:

public CONST PostgreSQL94Platform = 'PostgreSQL94Platform';
....

public function getDatabaseType(): string;

Context

@ChristophWurst
Copy link
Member

I'd rather put this onto \OCP\IDBConnection to make it also usable outside the context of migrations.

This will be a solid replacement for the leaky \OCP\IDBConnection::getDatabasePlatform. Ref

* @todo we are leaking a 3rdparty type here

@ChristophWurst ChristophWurst added 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Jan 27, 2022
miaulalala added a commit that referenced this issue Sep 14, 2023
To avoid leaking internals (OC), wrap the getDatabasePlatform and provide the
associated constants

fixes #30877

Signed-off-by: Anna Larch <anna@nextcloud.com>
miaulalala added a commit that referenced this issue Sep 18, 2023
To avoid leaking internals (OC), wrap the getDatabasePlatform and provide the
associated constants

fixes #30877

Signed-off-by: Anna Larch <anna@nextcloud.com>
miaulalala added a commit that referenced this issue Sep 19, 2023
To avoid leaking internals (OC), wrap the getDatabasePlatform and provide the
associated constants

fixes #30877

Signed-off-by: Anna Larch <anna@nextcloud.com>
zak39 pushed a commit to arawa/server that referenced this issue Oct 11, 2023
To avoid leaking internals (OC), wrap the getDatabasePlatform and provide the
associated constants

fixes nextcloud#30877

Signed-off-by: Anna Larch <anna@nextcloud.com>
zak39 pushed a commit to zak39/server that referenced this issue Dec 19, 2023
To avoid leaking internals (OC), wrap the getDatabasePlatform and provide the
associated constants

fixes nextcloud#30877

Signed-off-by: Anna Larch <anna@nextcloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of enhancement technical debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants