Association query #19

Brooooooklyn · 2017-08-08T07:30:14Z

Association query

refactor(PredicateProvider): PredicateProvider now support multiple tables, prepare for Association query
feat(PredicateProvider): support deep nested association query
feat(Database): support association query without association fields
test(PredicateProvider): add test for tableName missmatch in PredicateProvider
feat(Database): support nested predicate key
fix(Database#remove): stop passing Clause to PredicateProvider, instead of passing Predicate
chore: display report coverage after run coverage
feat(Database): warn if delete/remove/update all table
feat(utils/keys): use keys as alias for Object.keys
chore: yarn upgrade

Breaking Changes

feat(Database#get): restrict Associated Field position in Fields
feat(Database#get): Only navigation properties in query now valid

Related Issue:

resolve #14

coveralls · 2017-08-08T07:33:19Z

Coverage increased (+0.06%) to 96.4% when pulling eb765b1d323d0f3ae6d5ce714014cd3097ac38da on feature/association-query into ce214b2 on master.

coveralls · 2017-08-08T07:38:31Z

Coverage increased (+0.06%) to 96.4% when pulling 8c97a5bf0bd34d179e98a2bd0138b8eaa394ac07 on feature/association-query into ce214b2 on master.

Saviio · 2017-08-08T12:24:20Z

这个是已经实现好了呀？

Saviio · 2017-08-09T02:40:42Z

@chuan6 @Miloas 之后你们先帮我review下这个PR啦~~

chuan6 · 2017-08-09T03:06:54Z

@Saviio 嗯嗯，代码看过，但还没理解这个 PR 要达到的效果，今天再看看 👌

Brooooooklyn · 2017-08-09T03:26:25Z

达到的效果是:

transform

{
  fields: [....],
  where: {
    'creator.gender': 'male' 
  }
}

to

SELECT 
  _id,
  Member.id,
  Member.name,
  Member.gender
FROM Message 
JOIN Member ON Message.creatorId = Member.id
WHERE Member.gender = 'male'

chuan6 · 2017-08-09T06:08:12Z

src/storage/Database.ts

@@ -445,7 +449,8 @@ export class Database {
        mainTable: table!
      }
      const { limit, skip } = clause
-      const provider = new PredicateProvider(table!, clause.where)
+      const tables = this.getAssoTablesDef(db, tableName, table)


@Brooooooklyn 这边 tableName 和 table 可能不一致吗？

chuan6 · 2017-08-09T07:11:21Z

@Brooooooklyn 我第一次注意到 createPredicate 这个函数，根据目前的实现，它的作用应该已经被现在的 PredicateProvider （可以处理空的 null 或 undefined 的 clause）代替了，还需要保留它吗？

Brooooooklyn · 2017-08-09T08:23:41Z

可以保留，配合 tryCatch 作为:

try {
  new PredicateProvider(xxxxx).getPredicate()
  // success logic
} catch (e) {
  // error logic
}

的语法糖

coveralls · 2017-08-09T10:33:26Z

Coverage decreased (-0.06%) to 96.28% when pulling 4ef39feb47ef967faaced5ca1755d3e79c178461 on feature/association-query into ce214b2 on master.

coveralls · 2017-08-09T10:38:54Z

Coverage increased (+0.08%) to 96.413% when pulling fa72de7d08cd9b45f2d53ed11108f4f8e0d630fa on feature/association-query into ce214b2 on master.

Brooooooklyn · 2017-08-09T10:40:35Z

test/specs/storage/Database.public.spec.ts

+        expect(result).to.deep.equal(innerTarget)
+      })
+
+      it.skip('should get association by deep nested Association query without association fields', function* () {


need some help here @Saviio

蛤? 有啥问题

Saviio · 2017-08-09T10:46:43Z

src/storage/Database.ts

@@ -747,6 +759,22 @@ export class Database {
    }
  }

+  private getAssoTablesDef(db: lf.Database, tableName: string, defaultTable: null | lf.schema.Table = null) {


getRelatedTables

Brooooooklyn · 2017-08-09T10:51:38Z

我现在的做法是:

在 JoinInfo 中构建一个 tablesStructure, 它是这样的类型:

{
  [index: string]: { 
    table: lf.schema.Table
    contextName: string
  }
}

使 buildSelector 中 predicateProvider.getPredicate() 变成 lazy 的，即: 不是在 build joinInfo 的时候就 getPredicate，而是在从 joinInfo 中 build joinQuery 的时候再 getPredicate , 这个时候可以拿到整条查询语句的 tablesStructure，比如:

{
  'Task#1': {
     table: tableClass, // alias Task#1,
     contextName: 'Task#1'
   },
   'Task#1@project': {
      table: tableClass, // alias Project#1
      contextName: Project#1
    },
    'Project#1@organization': {
      table: tableClass, // alias Organization#1
      contextName: Organization#1
    }
}

让一条 get/upsert 语句中所有的 joinPredicate 和 wherePredicate 共用这个 tablesStructure, 然后到 PredicateProvider 中根据 meta 解析出 predicate

这样做有一个问题就是，当 query 的 predicate 中指定了某个关联的条件，但 fields 中并不包含这个关联的时候会构建不出 tablesStructure, 比如:

Database.get('Task', {
  fields:  ['_id'],
  where: {
    'project.organization._id': 'xxxx'
  }
})

具体表现就是 skip 掉的那个测试

coveralls · 2017-08-09T10:56:38Z

Coverage increased (+0.08%) to 96.413% when pulling 49f780161e544c70a93f43884125f359b9b04515 on feature/association-query into ce214b2 on master.

Brooooooklyn · 2017-08-09T15:10:29Z

ready

coveralls · 2017-09-06T03:23:41Z

Coverage decreased (-0.08%) to 96.266% when pulling 77e98bd on feature/association-query into a7e2496 on master.

coveralls · 2017-09-06T04:06:17Z

Coverage increased (+0.2%) to 96.504% when pulling 68fb8fe on feature/association-query into a7e2496 on master.

coveralls · 2017-09-06T07:44:23Z

Coverage increased (+0.4%) to 96.7% when pulling 541b338 on feature/association-query into a7e2496 on master.

coveralls · 2017-09-06T10:46:50Z

Coverage increased (+0.4%) to 96.7% when pulling 1bdb3a0 on feature/association-query into a7e2496 on master.

coveralls · 2017-09-07T07:16:30Z

Coverage increased (+0.1%) to 96.487% when pulling ac32a6d on feature/association-query into a7e2496 on master.

Saviio · 2017-09-08T15:17:46Z

test/specs/storage/Database.public.spec.ts

+        })
+      })
+
+      it('should warn if build addition join info from predicate failed', function* () {


typo additional

Saviio · 2017-09-08T15:18:42Z

test/specs/storage/Database.public.spec.ts

@@ -1049,9 +1205,7 @@ export default describe('Database Testcase: ', () => {
      const execRet1 = yield database.upsert<ProgramSchema>('Program', program)

      yield database.delete<ProgramSchema>('Program', {


这里接口不对称真的好吗

ok 的，get 才需要 fields，skip limit order 这些

我明白，但是我还认为应该把接口设计的更对称，从形式上更接近

DELETE FROM table WHERE id > 0

这里的省略是一种"习惯的破坏"

Saviio · 2017-09-08T15:19:23Z

test/specs/storage/Database.public.spec.ts

@@ -539,21 +695,6 @@ export default describe('Database Testcase: ', () => {
          })
      })

-      it('should throw if only navigator was included in query', function* () {


咦，为什么删了

行为变了，默认会在所有的 associated query 上加上 pk

Saviio · 2017-09-08T15:25:37Z

test/specs/storage/Database.public.spec.ts

+        expect(result.project.organization._id).to.equal(innerTarget.project._organizationId)
+      })
+
+      it('should merge fields when get value by deep nested Association query with nested association fields', function* () {


should merge fields if a nested association in the WHERE clause

Saviio

剩下的明天再看，脑子转不动了

… to merge query.fields & fields built from predicate

…predicate reduce the logics in Database#buildSelector and PredicateProvider#normalizeMeta

…interface names

feat(Database): Only navigation properties in query now valid

coveralls · 2017-09-08T16:00:10Z

Coverage increased (+0.1%) to 96.487% when pulling d78d3a1 on feature/association-query into a7e2496 on master.

Saviio · 2017-09-13T07:49:00Z

src/storage/Database.ts

@@ -745,6 +785,115 @@ export class Database {
    }
  }

+  private buildTablesStructure(defaultTable: lf.schema.Table, aliasName?: string, tablesStruct: TablesStruct = Object.create(null)) {


这个函数叫 tablesStructure 有点奇怪....."复数的表"的结构，我觉得这系列函数名/变量的命名不如就叫tableStruct

coveralls · 2017-09-14T08:53:37Z

Coverage increased (+0.2%) to 96.532% when pulling ba7a13a on feature/association-query into a7e2496 on master.

coveralls · 2017-09-14T10:12:59Z

Coverage increased (+0.2%) to 96.532% when pulling 9e7db4a on feature/association-query into a7e2496 on master.

Brooooooklyn requested a review from Saviio August 8, 2017 07:30

Brooooooklyn changed the title ~~Association query~~ [WIP]Association query Aug 8, 2017

Brooooooklyn force-pushed the feature/association-query branch from 9153b62 to eb765b1 Compare August 8, 2017 07:31

Brooooooklyn force-pushed the feature/association-query branch from eb765b1 to 8c97a5b Compare August 8, 2017 07:37

Saviio requested review from chuan6 and Miloas August 8, 2017 12:24

chuan6 reviewed Aug 9, 2017

View reviewed changes

Brooooooklyn force-pushed the feature/association-query branch 2 times, most recently from 385b4b8 to 4ef39fe Compare August 9, 2017 10:31

Brooooooklyn force-pushed the feature/association-query branch from 4ef39fe to fa72de7 Compare August 9, 2017 10:37

Brooooooklyn commented Aug 9, 2017

View reviewed changes

Saviio reviewed Aug 9, 2017

View reviewed changes

Brooooooklyn force-pushed the feature/association-query branch from fa72de7 to 49f7801 Compare August 9, 2017 10:55

Brooooooklyn force-pushed the feature/association-query branch 3 times, most recently from 4877b5e to cdafc68 Compare August 9, 2017 15:09

Brooooooklyn changed the title ~~[WIP]Association query~~ Association query Aug 9, 2017

Brooooooklyn force-pushed the feature/association-query branch from 5f161a0 to 77e98bd Compare September 6, 2017 03:13

Brooooooklyn force-pushed the feature/association-query branch from 77e98bd to 68fb8fe Compare September 6, 2017 04:04

Brooooooklyn force-pushed the feature/association-query branch from 68fb8fe to 541b338 Compare September 6, 2017 07:43

Brooooooklyn force-pushed the feature/association-query branch from 274b5e4 to ac32a6d Compare September 7, 2017 07:09

Saviio reviewed Sep 8, 2017

View reviewed changes

Brooooooklyn added 6 commits September 8, 2017 23:56

refactor(Database): introduce merge-fields in utils, use merge fields…

46b3fa0

… to merge query.fields & fields built from predicate

test(TaskSchema): add tasklist schema, add more nested predicate case

4509849

refactor(PredicateProvidor): use new parse-predicate helper to parse …

b93e9d3

…predicate reduce the logics in Database#buildSelector and PredicateProvider#normalizeMeta

chore: yarn upgrade

2ccf2ee

docs(QueryDescription): update docs, add fields description & update …

a07455d

…interface names

chore: code cleanup

d78d3a1

feat(Database): Only navigation properties in query now valid

Brooooooklyn force-pushed the feature/association-query branch from ac32a6d to d78d3a1 Compare September 8, 2017 15:58

Saviio reviewed Sep 13, 2017

View reviewed changes

chore: fix code-style

9e7db4a

Saviio force-pushed the feature/association-query branch from ba7a13a to 9e7db4a Compare September 14, 2017 10:11

chuan6 added the Feature label Nov 17, 2017

chuan6 self-assigned this Nov 17, 2017

Brooooooklyn closed this Aug 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Association query #19

Association query #19

Brooooooklyn commented Aug 8, 2017 •

edited

Loading

coveralls commented Aug 8, 2017

coveralls commented Aug 8, 2017

Saviio commented Aug 8, 2017

Saviio commented Aug 9, 2017

chuan6 commented Aug 9, 2017

Brooooooklyn commented Aug 9, 2017

chuan6 Aug 9, 2017

chuan6 commented Aug 9, 2017

Brooooooklyn commented Aug 9, 2017

coveralls commented Aug 9, 2017

coveralls commented Aug 9, 2017

Brooooooklyn Aug 9, 2017

Saviio Aug 9, 2017

Saviio Aug 9, 2017

Brooooooklyn commented Aug 9, 2017 •

edited

Loading

coveralls commented Aug 9, 2017

Brooooooklyn commented Aug 9, 2017

coveralls commented Sep 6, 2017

coveralls commented Sep 6, 2017

coveralls commented Sep 6, 2017

coveralls commented Sep 6, 2017

coveralls commented Sep 7, 2017

Saviio Sep 8, 2017

Saviio Sep 8, 2017 •

edited

Loading

Brooooooklyn Sep 8, 2017

Saviio Sep 13, 2017 •

edited

Loading

Saviio Sep 8, 2017

Brooooooklyn Sep 8, 2017

Saviio Sep 8, 2017

Saviio left a comment

coveralls commented Sep 8, 2017

Saviio Sep 13, 2017 •

edited

Loading

coveralls commented Sep 14, 2017

coveralls commented Sep 14, 2017

		@@ -1049,9 +1205,7 @@ export default describe('Database Testcase: ', () => {
		const execRet1 = yield database.upsert<ProgramSchema>('Program', program)

		yield database.delete<ProgramSchema>('Program', {

Association query #19

Association query #19

Conversation

Brooooooklyn commented Aug 8, 2017 • edited Loading

Association query

Breaking Changes

Related Issue:

coveralls commented Aug 8, 2017

coveralls commented Aug 8, 2017

Saviio commented Aug 8, 2017

Saviio commented Aug 9, 2017

chuan6 commented Aug 9, 2017

Brooooooklyn commented Aug 9, 2017

Choose a reason for hiding this comment

chuan6 commented Aug 9, 2017

Brooooooklyn commented Aug 9, 2017

coveralls commented Aug 9, 2017

coveralls commented Aug 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Brooooooklyn commented Aug 9, 2017 • edited Loading

coveralls commented Aug 9, 2017

Brooooooklyn commented Aug 9, 2017

coveralls commented Sep 6, 2017

coveralls commented Sep 6, 2017

coveralls commented Sep 6, 2017

coveralls commented Sep 6, 2017

coveralls commented Sep 7, 2017

Choose a reason for hiding this comment

Saviio Sep 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Saviio Sep 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Saviio left a comment

Choose a reason for hiding this comment

coveralls commented Sep 8, 2017

Saviio Sep 13, 2017 • edited Loading

Choose a reason for hiding this comment

coveralls commented Sep 14, 2017

coveralls commented Sep 14, 2017

Brooooooklyn commented Aug 8, 2017 •

edited

Loading

Brooooooklyn commented Aug 9, 2017 •

edited

Loading

Saviio Sep 8, 2017 •

edited

Loading

Saviio Sep 13, 2017 •

edited

Loading

Saviio Sep 13, 2017 •

edited

Loading