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: Irrelevant entities fix #78

Open
wants to merge 10 commits into
base: development
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/polaris-common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,6 @@
"tslint-consistent-codestyle": "^1.16.0",
"tslint-eslint-rules": "^5.4.0",
"tslint-plugin-prettier": "^2.3.0",
"typescript": "^4.0.2"
"typescript": "^4.2.3"
}
}
2 changes: 1 addition & 1 deletion packages/polaris-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"tslint-consistent-codestyle": "^1.16.0",
"tslint-eslint-rules": "^5.4.0",
"tslint-plugin-prettier": "^2.3.0",
"typescript": "^4.0.2",
"typescript": "^4.2.3",
"ws": "^7.3.1"
}
}
2 changes: 1 addition & 1 deletion packages/polaris-graphql-logger/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@
"tslint-consistent-codestyle": "^1.16.0",
"tslint-eslint-rules": "^5.4.0",
"tslint-plugin-prettier": "^2.3.0",
"typescript": "^4.0.2"
"typescript": "^4.2.3"
}
}
2 changes: 1 addition & 1 deletion packages/polaris-middlewares/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,6 @@
"tslint-consistent-codestyle": "^1.16.0",
"tslint-eslint-rules": "^5.4.0",
"tslint-plugin-prettier": "^2.3.0",
"typescript": "^4.0.2"
"typescript": "^4.2.3"
}
}
10 changes: 5 additions & 5 deletions packages/polaris-nest/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@
],
"dependencies": {
"@enigmatis/polaris-core": "^2.0.0-beta.66",
"@nestjs/common": "^7.6.12",
"@nestjs/core": "^7.6.12",
"@nestjs/graphql": "^7.9.9",
"@nestjs/platform-express": "^7.6.12",
"@nestjs/common": "^7.6.14",
"@nestjs/core": "^7.6.14",
"@nestjs/graphql": "^7.10.2",
"@nestjs/platform-express": "^7.6.14",
"@nestjs/typeorm": "^7.1.5",
"apollo-server": "^2.17.0",
"apollo-server-plugin-base": "0.6.10",
Expand Down Expand Up @@ -65,7 +65,7 @@
"tslint-config-prettier": "^1.18.0",
"tslint-consistent-codestyle": "^1.16.0",
"tslint-eslint-rules": "^5.4.0",
"typescript": "^4.0.2"
"typescript": "^4.2.3"
},
"peerDependencies": {
"@nestjs/common": "^6.7.0 || ^7.0.0",
Expand Down
1 change: 1 addition & 0 deletions packages/polaris-nest/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"baseUrl": "./",
"incremental": true,
"skipLibCheck": true,
"noImplicitAny": false,
"strictPropertyInitialization": false,
"lib": [
"es2020.bigint"
Expand Down
2 changes: 1 addition & 1 deletion packages/polaris-permissions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@
"tslint-consistent-codestyle": "^1.16.0",
"tslint-eslint-rules": "^5.4.0",
"tslint-plugin-prettier": "^2.3.0",
"typescript": "^4.0.2"
"typescript": "^4.2.3"
}
}
2 changes: 1 addition & 1 deletion packages/polaris-schema/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@
"tslint-consistent-codestyle": "^1.16.0",
"tslint-eslint-rules": "^5.4.0",
"tslint-plugin-prettier": "^2.3.0",
"typescript": "^4.0.2"
"typescript": "^4.2.3"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class AuthorService {
}

public async createManyAuthors(): Promise<boolean> {
for (let i = 0; i < 15; i++) {
for (let i = 0; i < 10; i++) {
const author = new Author(`Ron${i}`, 'Katz');
await this.authorRepository.save(author);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class AuthorService {
}

public async createManyAuthors(): Promise<boolean> {
for (let i = 0; i < 15; i++) {
for (let i = 0; i < 10; i++) {
const author = new Author(`Ron${i}`, 'Katz');
await this.authorRepository.save(author);
}
Expand Down
10 changes: 5 additions & 5 deletions packages/polaris-test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@
"dependencies": {
"@enigmatis/polaris-core": "^2.0.0-beta.66",
"@enigmatis/polaris-nest": "^1.9.0",
"@nestjs/common": "^7.6.12",
"@nestjs/core": "^7.6.12",
"@nestjs/graphql": "^7.9.9",
"@nestjs/platform-express": "^7.6.12",
"@nestjs/common": "^7.6.14",
"@nestjs/core": "^7.6.14",
"@nestjs/graphql": "^7.10.2",
"@nestjs/platform-express": "^7.6.14",
"@nestjs/typeorm": "^7.1.5",
"@types/pg": "^7.14.5",
"apollo-server": "^2.17.0",
Expand Down Expand Up @@ -78,7 +78,7 @@
"tslint-consistent-codestyle": "^1.16.0",
"tslint-eslint-rules": "^5.4.0",
"tslint-plugin-prettier": "^2.3.0",
"typescript": "^4.0.2"
"typescript": "^4.2.3"
},
"peerDependencies": {
"@nestjs/common": "^6.7.0 || ^7.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/polaris-test/server/schema/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ export const resolvers = {
args: any,
context: PolarisGraphQLContext,
): Promise<boolean> => {
for (let i = 1; i <= 15; i++) {
for (let i = 0; i < 10; i++) {
const connection = getPolarisConnectionManager().get(process.env.SCHEMA_NAME);
const authorRepo = connection.getRepository(Author, context);
const newAuthor = new Author(`Ron${i}`, `Katz`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const extractRelevantIrrelevantEntitiesByQuery = (query: string, irrelevantEntit
}
};

describe('online pagination tests - left outer join implementation', () => {
describe('online pagination tests', () => {
test.each(createServersWithInnerAndLeftJoin())(
'fetch authors, page-size and data version sent, return accordingly',
async (server, query) => {
Expand Down Expand Up @@ -251,4 +251,25 @@ describe('online pagination tests - left outer join implementation', () => {
});
},
);
test.each(createServersWithInnerAndLeftJoin())(
'fetch authors, entities with a specific data-version are not returned as irrelevant entities in a following page with the same data-version',
async (server, query) => {
await polarisTest(server, async () => {
await graphqlRawRequest(createManyAuthors.request, {}, {});
let result = await graphqlRawRequest(
query,
{ 'page-size': 5, 'data-version': 1 },
{},
);
const lastIdInDv = result.extensions.lastIdInDataVersion;
const lastDv = result.extensions.lastDataVersionInPage;
result = await graphqlRawRequest(
query,
{ 'page-size': 5, 'data-version': lastDv, 'last-id-in-dv': lastIdInDv },
{},
);
expect(result.extensions.irrelevantEntities).toBeUndefined();
});
},
);
});
2 changes: 1 addition & 1 deletion packages/polaris-typeorm/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@
"tslint-consistent-codestyle": "^1.16.0",
"tslint-eslint-rules": "^5.4.0",
"tslint-plugin-prettier": "^2.3.0",
"typescript": "^4.0.2"
"typescript": "^4.2.3"
}
}
91 changes: 71 additions & 20 deletions packages/polaris-typeorm/src/handlers/data-version-handler.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { PolarisExtensions, PolarisGraphQLContext } from '@enigmatis/polaris-common';
import { EntityMetadata } from 'typeorm';
import { Brackets, EntityMetadata, QueryRunner, EntityTarget } from 'typeorm';
import { RelationMetadata } from 'typeorm/metadata/RelationMetadata';
import { DataVersion, PolarisConnection, PolarisEntityManager, SelectQueryBuilder } from '..';
import { isDescendentOfCommonModel } from '../utils/descendent-of-common-model';
import { setWhereCondition } from '../utils/query-builder-util';
import { cloneDeep } from 'lodash';
import { FindHandler } from './find-handler';

export class DataVersionHandler {
public async updateDataVersion<Entity>(
Expand Down Expand Up @@ -182,22 +184,33 @@ function joinDataVersionRelations(
return qb;
}

function getOrDataVersionCondition(
qb: SelectQueryBuilder<any>,
dataVersion: number,
names: string[],
) {
return new Brackets((qb2) => {
qb2.where(`${qb.alias}.dataVersion > :dataVersion`, { dataVersion });
names = names.slice(1);
for (const name of names) {
qb2 = qb2.orWhere(`${name}.dataVersion > :dataVersion`);
}
});
}

function applyDataVersionWhereConditions(
context: PolarisGraphQLContext,
qb: SelectQueryBuilder<any>,
names: string[],
shouldLoadRelations: boolean,
): SelectQueryBuilder<any> {
let dataVersion = context.requestHeaders.dataVersion || 0;
const lastIdInDataVersion = context.requestHeaders.lastIdInDV;
if (lastIdInDataVersion) {
if (lastIdInDataVersion && shouldLoadRelations) {
dataVersion--;
}
if (dataVersion > 0) {
qb.where(`${qb.alias}.dataVersion > :dataVersion`, { dataVersion });
names = names.slice(1);
for (const name of names) {
qb = qb.orWhere(`${name}.dataVersion > :dataVersion`);
}
qb = setWhereCondition(qb, getOrDataVersionCondition(qb, dataVersion, names));
}
return qb;
}
Expand Down Expand Up @@ -226,7 +239,7 @@ export const leftJoinDataVersionFilter = (
entityMetadata,
names,
);
return applyDataVersionWhereConditions(context, qb, names);
return applyDataVersionWhereConditions(context, qb, names, shouldLoadRelations);
}
return qb;
};
Expand All @@ -235,11 +248,17 @@ export const InnerJoinDataVersionQuery = (
connection: PolarisConnection,
context: PolarisGraphQLContext,
rootEntityMetadata: EntityMetadata,
): string => {
criteria: any,
entityClass: EntityTarget<any>,
queryRunner?: QueryRunner,
): { query: string; parameters: any } => {
const selectQueriesMap: Map<string, SelectQueryBuilder<any>> = createEntitiesSelectQueries(
connection,
rootEntityMetadata,
context,
criteria,
entityClass,
queryRunner,
);

const rootEntityIdSelection = `"${rootEntityMetadata.tableName}"."id"`;
Expand All @@ -249,10 +268,12 @@ export const InnerJoinDataVersionQuery = (
function buildInnerJoinQuery(
selectQueries: SelectQueryBuilder<any>[],
rootEntityIdSelection: string,
): string {
): { query: string; parameters: any } {
let finalQuery = 'WITH w1(id, dv) AS (';
const union = ' UNION ';
const parameters: any[] = [];
selectQueries.forEach((query: SelectQueryBuilder<any>) => {
parameters.push(...selectQueries[0].getQueryAndParameters()[1]);
const splitQuery = query.getSql().split('SELECT');
finalQuery = finalQuery.concat(`SELECT ${rootEntityIdSelection},`);
finalQuery =
Expand All @@ -263,17 +284,27 @@ function buildInnerJoinQuery(
finalQuery = finalQuery.concat(
') SELECT w1.id AS "entityId", MAX(w1.dv) AS "maxDV" FROM w1 GROUP BY w1.id ORDER BY "maxDV", "entityId"',
);
return finalQuery;
return { query: finalQuery, parameters };
}

function createEntitiesSelectQueries(
connection: PolarisConnection,
rootEntityMetadata: EntityMetadata,
context: PolarisGraphQLContext,
criteria: any,
entityClass: EntityTarget<any>,
queryRunner?: QueryRunner,
): Map<string, SelectQueryBuilder<any>> {
const selectQueries: Map<string, SelectQueryBuilder<any>> = new Map();

const rootEntitySelectQuery = getRootEntitySelectQuery(connection, rootEntityMetadata, context);
const rootEntitySelectQuery = getRootEntitySelectQuery(
connection,
rootEntityMetadata,
context,
entityClass,
queryRunner,
criteria,
);
selectQueries.set(rootEntityMetadata.tableName, rootEntitySelectQuery);

createChildEntitiesSelectQueries(
Expand All @@ -282,6 +313,7 @@ function createEntitiesSelectQueries(
context.dataVersionContext!.mapping!,
selectQueries,
rootEntityMetadata.tableName,
criteria,
);

return selectQueries;
Expand All @@ -291,12 +323,23 @@ function getRootEntitySelectQuery(
connection: PolarisConnection,
rootEntityMetadata: EntityMetadata,
context: PolarisGraphQLContext,
entityClass: EntityTarget<any>,
queryRunner?: QueryRunner,
criteria?: any,
): SelectQueryBuilder<any> {
const rootEntityQueryBuilder = connection.createQueryBuilder();
setWhereClauseOfQuery(rootEntityQueryBuilder, context, rootEntityMetadata);
return rootEntityQueryBuilder
.addSelect(`${rootEntityMetadata.tableName}.dataVersion`)
.addFrom(rootEntityMetadata.tableName, rootEntityMetadata.tableName);
const rootEntityQueryBuilder = connection.createQueryBuilder(
entityClass,
rootEntityMetadata.tableName,
queryRunner,
);
setWhereClauseOfQuery(
rootEntityQueryBuilder,
context,
rootEntityMetadata,
rootEntityMetadata,
criteria,
);
return rootEntityQueryBuilder.select(`${rootEntityMetadata.tableName}.dataVersion`);
}

function createChildEntitiesSelectQueries(
Expand All @@ -305,6 +348,7 @@ function createChildEntitiesSelectQueries(
mapping: Map<string, any>,
selectQueries: Map<string, SelectQueryBuilder<any>>,
currentEntityPath: string,
criteria?: any,
): void {
if (entityMetadata.relations && mapping.size > 0) {
for (const relation of entityMetadata.relations) {
Expand All @@ -315,9 +359,11 @@ function createChildEntitiesSelectQueries(
const relationQueryBuilder: SelectQueryBuilder<any> = cloneDeep(parentSelectQuery!);
handleInnerJoinForRelation(
relationQueryBuilder,
entityMetadata,
relationEntityMetadata,
relation,
context,
criteria,
);
const entityPath = `${currentEntityPath}.${relationEntityMetadata.tableName}`;
selectQueries.set(entityPath, relationQueryBuilder);
Expand All @@ -335,11 +381,13 @@ function createChildEntitiesSelectQueries(

function handleInnerJoinForRelation(
queryBuilder: SelectQueryBuilder<any>,
rootEntityMetadata: EntityMetadata,
entityMetadata: EntityMetadata,
relation: RelationMetadata,
context: PolarisGraphQLContext,
criteria?: any,
) {
setWhereClauseOfQuery(queryBuilder, context, entityMetadata);
setWhereClauseOfQuery(queryBuilder, context, rootEntityMetadata, entityMetadata, criteria);
if (relation.relationType === 'one-to-many' || relation.relationType === 'many-to-one') {
handleInnerJoinForOneToManyRelation(queryBuilder, entityMetadata, relation);
} else if (relation.relationType === 'many-to-many') {
Expand Down Expand Up @@ -428,16 +476,19 @@ function getInnerJoinConditionByEntityMetadata(
function setWhereClauseOfQuery(
queryBuilder: SelectQueryBuilder<any>,
context: PolarisGraphQLContext,
rootEntityMetadata: EntityMetadata,
entityMetadata: EntityMetadata,
criteria?: any,
) {
let dataVersionThreshold = context.requestHeaders.dataVersion || 0;
if (context.requestHeaders.lastIdInDV) {
dataVersionThreshold--;
}
const realityIdThreshold = context.requestHeaders.realityId || 0;
queryBuilder.where(`${entityMetadata.tableName}.dataVersion > ${dataVersionThreshold}`);
queryBuilder.andWhere(`${entityMetadata.tableName}.realityId = ${realityIdThreshold}`);
queryBuilder.andWhere(`${entityMetadata.tableName}.deleted = false`);
queryBuilder.andWhere(`${rootEntityMetadata.tableName}.realityId = ${realityIdThreshold}`);
queryBuilder.andWhere(`${rootEntityMetadata.tableName}.deleted = false`);
FindHandler.applyUserConditions(criteria, queryBuilder);
}

function getChildDVMapping(
Expand Down
Loading