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

fix: Capacitor driver PRAGMA requests failing on Android #7728

Merged
merged 9 commits into from Jun 24, 2021

Conversation

chriswep
Copy link
Contributor

@chriswep chriswep commented Jun 9, 2021

  • fix: PRAGMA needs to be run through the query method, since it returns data (Android sqlite rejects the request otherwise)
  • added WAL journal mode

@imnotjames
Copy link
Contributor

Just to confirm - there's no way of testing this, is there? Aside from running it on Android.

@chriswep
Copy link
Contributor Author

Just to confirm - there's no way of testing this, is there? Aside from running it on Android.

i don't think there is a reasonable way within the scope of this project. if i didn't overlook something, i also don't see other drivers testing something like that (or test anything at all..). probably another reason to have plugin drivers in the future?

@pleerock
Copy link
Member

if i didn't overlook something, i also don't see other drivers testing something like that

correct and that's why when somebody changes something and break something on these drivers - we don't even realize it. Then people have different issues they report. If we can automatize test running on this (or any other which isn't automatized) - it will save us a lot from many issues. And this question is not related to the future - automatization happens once no matter how project is structured.

@chriswep
Copy link
Contributor Author

chriswep commented Jun 15, 2021

@pleerock i agree. fwiw, i was bitten by this myself more than once after upgrading to a new TypeORM release. however in those cases the issues were changes outside the direct scope of the driver. So i guess what i was trying to say: moving drivers out of core and importing them as plugins prevents them from breaking other stuff on a conceptual level. furthermore it makes them easier to test. but i guess that is something we would all agree on anyway.

as far as this PR is concerned, i think it wouldn't make sense to attempt testing this. this code doesn't affect other parts of TypeORM and this is about conforming to the SQLite spec in combination with the Android Interface in combination with the Capacitor SQLite plugin which i failed concerning the PRAGMA command in the initial implementation. if drivers would test at this level, they'd need to test against the whole spec of their respective engine in combination with every supported client (in my case, multiple iOS and Android versions). maybe a more achievable goal would be a general purpose test that tests each driver through the TypeORM spec?

@imnotjames imnotjames changed the title fix: Capacitor driver (PRAGMA requests failing on Android) fix: Capacitor driver PRAGMA requests failing Jun 15, 2021
@imnotjames imnotjames changed the title fix: Capacitor driver PRAGMA requests failing fix: Capacitor driver PRAGMA requests failing on Android Jun 15, 2021
@imnotjames
Copy link
Contributor

Has this been tested on iOS? Will it break iOS' implementation?

@pleerock
Copy link
Member

@chriswep yes I don't mind a new testing setup to be part of this PR. However we need contributions from anybody in order to fix the issues we have.

@chriswep
Copy link
Contributor Author

Has this been tested on iOS? Will it break iOS' implementation?

yes it has been tested, no it doesn't break iOS @imnotjames

@imnotjames imnotjames merged commit 9620a26 into typeorm:master Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants