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.

Sign up for GitHub

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

Jump to bottom

Pull request for issues 11 and 13. #19

Open

tfrank7415 wants to merge 6 commits into nwestfall:master from tfrank7415:master

tfrank7415 commented Dec 30, 2022 •

edited

Loading

Additions:

StoreSchemaUpgrade.cs added. This class allows users to configure store schema upgrades via the service builder in Program.cs.
IndexedDbUpgradeFunctions.cs added. This class defines what different upgrade actions the user can perform.
Slight change to blazorDB.createDB() to check if the user wants to perform a schema upgrade. Code is commented, so you can review.
10 functions added to blazorDB.js to perform the schema upgrades.
Updated example project.
5 functions added to IndexDbManager.cs and blazorDB.js to address issue 13. New functions allow the user retrieve the first item in a store, the last item in a store, the min index for a store, the max index for a store, and the count of the records in a store.

tyler.franklin added 2 commits

December 29, 2022 19:32


          Check-in for Issue 11. 10 functions added to blazorDb.js that allow u…

f3cde97

…sers to make schema upgrades.


          Cleaning up excess code in Program.cs.

a20b6f2

Author

tfrank7415 commented Dec 30, 2022

Working on issue 13 next. Let me know if you have any coding preferences, so I can implement them on my next pull request.


          5 new functions added to address issue nwestfall#13.

8c89ae4

tfrank7415 changed the title ~~Pull request for issue 11.~~ Pull request for issues 11 and 13.


          Added comments.

04e6f6d

nwestfall requested changes

View reviewed changes

Owner

nwestfall left a comment

Nice! Overall, this is really solid. Let me know what you think of the comments and we can go from there.

src/BlazorDB/StoreSchemaUpgrade.cs Outdated Show resolved Hide resolved

src/BlazorDB/DbStore.cs Outdated Show resolved Hide resolved

src/BlazorDB/IndexedDbUpgradeFunctions.cs Outdated Show resolved Hide resolved

src/BlazorDB/wwwroot/blazorDB.js Outdated

+                          window.blazorDB.getTable(item.dbName, item.storeName).then(table => {
+                              table.toCollection().count().then((result) => {
+                                  dotnetReference.invokeMethodAsync('BlazorDBCallback', transaction, false, 'getMinIndex succeeded');
+                                  resolve(result == 0 ? "Table empty" : 0);

Owner

nwestfall Jan 2, 2023

What about returning a -1 instead of Table Empty?

The concern is just getting it the same type return for if/else

Author

tfrank7415 Jan 2, 2023

Implemented.

src/BlazorDB/wwwroot/blazorDB.js Outdated

+                          window.blazorDB.getTable(item.dbName, item.storeName).then(table => {
+                              table.toCollection().count().then((result) => {
+                                  dotnetReference.invokeMethodAsync('BlazorDBCallback', transaction, false, 'getMaxIndex succeeded');
+                                  resolve(result > 0 ? (result - 1) : "Table empty");

Owner

nwestfall Jan 2, 2023

What about returning a -1 instead of Table Empty?

The concern is just getting it the same type return for if/else

Author

tfrank7415 Jan 2, 2023

Implemented.

src/BlazorDB/wwwroot/blazorDB.js

Comment on lines +289 to +291

+                  // Assumption. Only one schema update per version update
+                  // Will allow multiple updates after proof of concept
+                  // This function purposefully DOES NOT return a promise. This is because .upgrade() DOES NOT return a promise.

Owner

nwestfall Jan 2, 2023

Just noting this assumption.

I'm okay for a round 1 draft too, it's just one update per version and we just error if storeSchemeUpgrades.length > 1 to let the user know.

src/BlazorDB/wwwroot/blazorDB.js Outdated Show resolved Hide resolved

src/BlazorDB/wwwroot/blazorDB.js

+                  // Assumption. Only one schema update per version update
+                  // Will allow multiple updates after proof of concept
+                  // This function purposefully DOES NOT return a promise. This is because .upgrade() DOES NOT return a promise.
+                  upgradeDbSchema: function (db, stores, dbStore) {

Owner

nwestfall Jan 2, 2023

This is a great add, only thing I'd look at doing is handle some possible user errors. Some that come to mind

Column in row doesn't exist
If the column for the row returns a null value how does split or other actions perform?

Just looking to see how we support these cases

Author

tfrank7415 Jan 2, 2023

Working this now. I'll provide more feedback later tonight or tomorrow.

Author

tfrank7415 Jan 3, 2023

Currently, all errors are caught in the catch at line 63 in blazorDB.js. So if I try to split a row that does not exist, the following error is logged to the console.

We need to make these errors more specific and useful. I think the best way to do that is to create an error handling function and call it inside of upgradeDbSchema(). The error handling function will look at the schemaUpdate.upgradeAction property and perform different checks to ensure all properties contain useable data. If a property does not contain useful data, I will throw an error message relative to that property. For instance, if the schemaUpdate.upgradeAction == 'split' and the columnsToPerformActionOn property is null or does not exist in the store I will throw an error that states, "columnsToPerformActionOn is null" or "columnsToPerformActionOn does not exist in store".

This thrown error will be caught at line 63 in blazorDB.js and logged to the console. The upgrade error will not effect the db instance since the version.upgrade() creates a database transaction. Dexie.js will simply use the last version of the database. So whatever the current version is.

What do you think? I think the custom error handling function will be the only way to return useful error messages.

Author

tfrank7415 Jan 4, 2023 •

edited

Loading

Change of plans after testing. I decided to add verifyStoreSchemaUpgrades() to the if logic that determines if upgradeDbSchema() is called. See below.

This ensures we will not attempt to perform a database upgrade via upgradeDbSchema() with incorrect parameters. I like this change because it ensures that the opening of a database will not be affected by any error that occurs in upgradeDbSchema().

verifyStoreSchemaUpgrade() will return false if the give storeSchemaUpgrade parameters do not match the given schemaUpdate.upgradeAction. See below for an example.

I will finish verifyStoreSchemaUpgrades() tomorrow then commit my code. Let me know if you have any feedback.

Owner

nwestfall Jan 9, 2023

I like this approach. Nice change.

Author

tfrank7415 Jan 10, 2023

Sweet. Let me know if you see anything else I need to change on my end.

BlazorDB.sln

    
              MinimumVisualStudioVersion = 10.0.40219.1

              Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "BlazorDB", "src\BlazorDB\BlazorDB.csproj", "{D0180843-D92A-44B7-9893-F2450415F208}"

              EndProject

              Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "BlazorDB.Example", "example\BlazorDB.Example\BlazorDB.Example.csproj", "{155CD782-6CEF-4115-9A04-1864A2CDFC90}"

              Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "BlazorDB.Example", "BlazorDB.Example\BlazorDB.Example.csproj", "{A47C9FCB-70BC-4B16-B5BB-68C8F6B1012E}"

Owner

nwestfall Jan 2, 2023

Just curious why we are moving this out of the example folder?

Author

tfrank7415 Jan 2, 2023

This was unintentional. Will be fixed on next commit.

tyler.franklin added 2 commits

January 2, 2023 14:07


          Requested changes for PR 19.

e1d2284


          Added verifyStoreSchemaUpgrades() to blazorDB.js.

ef727fc

Author

tfrank7415 commented Jan 5, 2023

Committed latest changes. Main change is that I added verifyStoreSchemaUpgrades() to blazorDB.js. I also added it to the if logic that determines if upgradeDbSchema() is called. See below.

verifyStoreSchemaUpgrades() will return false and console.error() if the following conditions occurs:

columnsToPerformActionOn, columnsToReceiveDataFromAction, and upgradeActionParameterList are not the expected length. This means they do not contain the correct number of column names.
columnsToPerformActionOn, columnsToReceiveDataFromAction, and upgradeActionParameterList are null or empty.

The above checks ensure that the database will always be opened even if the user creates an incorrect StoreSchemaUpgrade object in Program.cs.

Caveat: I could not verify that the column names passed by the property columnsToPerformActionOn were existing column names in a store. This is because I do not have access to Dexie.db.tables until after db.open() is called. Also, only indexed columns are stored in the store schema. So even if I had the ability to use db.tables, I would only be able to check the indexed columns to see if they existed.

tfrank7415 requested a review from nwestfall

January 16, 2023 18:13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet