storex icon indicating copy to clipboard operation
storex copied to clipboard

Add support for compound PK batch ops in test suite

Open poltak opened this issue 6 years ago • 3 comments

I'm not entirely sure if I'm going about this the correct way. I saw the empty should support batch operations with compound primary keys test and thought maybe that's what I should expand on (and which I did). Then I realized we'd need some new backend feature support flag for compound keys, so I added in compoundPrimaryKeys.

Though I noticed running the test suite from the root storex workspace directory, the should support batch operations with compound primary keys test runs for the firestore backend (which I haven't touched), so maybe I'm wrong about the need for a new feature support flag.

How far am I off the correct path here @ShishKabab ?

poltak avatar Jul 16 '19 03:07 poltak

Yeah, that's exactly right: that should be a feature flag. You can move the Firestore test here if it's easy to do, since I think I put it in there only to cover an immediate use case in a quick and dirty way.

One thing I noticed though, is that the primary key in your new tags collection is not defined as I'd expect. Isn't it usually done using something that looks life this?

{ field: ['name', 'email'], pk: true },

Or in the top of the collection definition:

pkIndex: ['name', 'email']

Have you checked whether the compound index is really created by Dexie for example?

ShishKabab avatar Jul 16 '19 08:07 ShishKabab

So I've had a look in the firestore repo and I don't actually see that test; I think I misunderstood that earlier in the test output (I was a bit rushed submitting this PR). As far as I can see running the test suite now, that test does not run for the firestore repo. Unless you know of the test existing in the firestore repo, let's ignore that.

The actual issue I have now is: When I go into the TypeORMStorageBackend (in the TypeORM submodule repo of storex-workspace) and add the new compoundPrimaryKeys flag, then run the test suite again (yarn test in the root of the storex-workspace repo), nothing changes and that test still does not run under the TypeORM StorageBackend tests suite.

Now inside the TypeORM submodule repo, from what I can see, it imports and resolves the @worldbrain/storex package from node_modules/@worldbrain/storex rather than the storex submodule that exists adjacent to the TypeORM submodule (i.e., where these changes exist - they do not exist in the node_modules/ version as they should have been downloaded from NPM).

I'm feeling like I'm not using the storex-workspace repo in the way it's intended to be used, or I'm missing some step/s to ensure the submodules are aware of each other rather than trying to use the packages hosted on NPM.

One thing I noticed though, is that the primary key in your new tags collection is not defined as I'd expect. Isn't it usually done using something that looks life this?

Yes I think you're right here. I've updated the commit in this PR.

poltak avatar Jul 21 '19 08:07 poltak

I think you're running into the issue that if you install any new package through yarn, the symlinks to the adjacent packages will get overwritten. You can confirm this by running ls -l node_modules/@worldbrain and not seeing any symlinks. Fix is to go to the storex-workspace root and running yarn bootstrap --ignore-scripts.

ShishKabab avatar Jul 22 '19 07:07 ShishKabab