Feature/data provider strategies #585

Dalantren · 2022-03-22T15:29:10Z

No description provided.

klumba12 · 2022-03-22T15:38:17Z

packages/qgrid-ngx-examples/src/examples/data-provider/example-data-provider.component.ts

@@ -21,23 +23,29 @@ export class ExampleDataProviderComponent {

  page$: Observable<Atom[]>;

+	dataProvider: DataProvider<Atom>;
+	gridModel: GridModel;


gridModel = this.qgrid.model();

klumba12 · 2022-03-22T15:38:30Z

packages/qgrid-ngx-examples/src/examples/data-provider/example-data-provider.component.ts

@@ -21,23 +23,29 @@ export class ExampleDataProviderComponent {

  page$: Observable<Atom[]>;

+	dataProvider: DataProvider<Atom>;


initialize dataPovider right here

make it private

can we again try to initialize dataProvider right here?

klumba12 · 2022-03-22T15:39:33Z

packages/qgrid-ngx-examples/src/examples/data-provider/example-data-provider.component.ts

+			RequestCountOnceStrategy,
+			CacheAlreadyRequestedPageStrategy,
+			ReverseDataStrategy,
+		], { server, gridModel: this.gridModel, pageSize: 50 });


remove pageSize here, use it from this.gridModel

const server = new FakeServer(this.dataService); new DataProvider<Atom>(this.gridModel, [ new RequestCountOnceStrategy(server), new CacheAlreadyRequestedPageStrategy(server, { backgroundPages: 2 }), new ReverseDataStrategy(), ]);

klumba12 · 2022-03-22T15:47:10Z

packages/qgrid-ngx-plugins/src/lib/data-provider/data-provider-strategy.ts

+	protected server: DataProviderServer<T>;
+	protected gridModel: GridModel;
+
+	processData(memo: T[], page?: number, size?: number): Observable<T[]> {


use gridModel to get page and size

packages/qgrid-ngx-plugins/src/lib/data-provider/strategies/request-count-once.ts

klumba12 · 2022-03-22T16:04:25Z

packages/qgrid-ngx-examples/src/examples/data-provider/example-data-provider.component.ts

+		this.gridModel = this.qgrid.model();
+
+		this.dataProvider = new DataProvider<Atom>([
+			RequestCountOnceStrategy,


RequestTotalCountOnceStategy

klumba12 · 2022-03-22T16:06:34Z

packages/qgrid-ngx-examples/src/examples/data-provider/example-data-provider.component.ts

+		this.dataProvider = new DataProvider<Atom>([
+			RequestCountOnceStrategy,
+			CacheAlreadyRequestedPageStrategy,
+			ReverseDataStrategy,


add BackgroundLoadPageStrategy(server, { numberOfPagesToLoad: 2 });

klumba12 · 2022-03-22T16:13:49Z

packages/qgrid-ngx-examples/src/examples/data-provider/example-data-provider.component.ts

 	}
 }

-class FakeServer {
+class FakeServer implements DataProviderServer<Atom> {
 	constructor(


@DataProviderCache() @PataProviderBackground() getPage() {} @DataProviderCache() @PataProviderBackground() getPage() {}

klumba12 · 2022-03-24T13:16:50Z

packages/qgrid-ngx-examples/src/examples/data-provider/example-data-provider.component.ts

@@ -21,23 +23,29 @@ export class ExampleDataProviderComponent {

  page$: Observable<Atom[]>;

+	dataProvider: DataProvider<Atom>;


make it private

klumba12 · 2022-03-24T13:19:45Z

packages/qgrid-ngx-examples/src/examples/data-provider/example-data-provider.component.ts

+
+class ReverseDataStrategy<T> extends DataProviderStrategy<T> {
+	processData(memo: T[]): Observable<T[]> {
+		return of(memo.slice().reverse());


why we need to do memo slice?

Rename it please to ExampleReverseDataStrategy

why we need to do memo slice?

.reverse() mutates initial array

klumba12 · 2022-03-24T13:23:16Z

packages/qgrid-ngx-plugins/src/lib/data-provider/data-provider-strategy.ts

+export abstract class DataProviderStrategy<T> {
+	protected gridModel: GridModel;
+
+	abstract processData(memo: T[]): Observable<T[]>;


make it as an interface with method

process(data: T[], context: { model: GridModel }): Observable<T[]>

still not sure about the signature with Observable

klumba12 · 2022-03-24T13:24:11Z

packages/qgrid-ngx-plugins/src/lib/data-provider/models.ts

+}
+
+export interface DataProviderServer<T> {
+	getPage(page?: number, size?: number): Observable<T[]>;


why page and size are optional?

we doesn't know how user will implement communication with server. Maybe it won't need any information about page or size

klumba12 · 2022-03-24T13:24:41Z

packages/qgrid-ngx-plugins/src/lib/data-provider/models.ts

+import { GridModel } from '@qgrid/ngx';
+import { Observable } from 'rxjs';
+
+export interface DataProviderOptions<T> {


why we need this interface?

klumba12 · 2022-03-24T13:29:10Z

packages/qgrid-ngx-plugins/src/lib/data-provider/strategies/request-count-once.ts

+			this.setPaginationCount(this.totalCount);
+			return of(memo);
+		}
+		return this.server.getTotal()


add new line

klumba12 · 2022-03-24T13:30:35Z

packages/qgrid-ngx-plugins/src/lib/data-provider/strategies/request-count-once.ts

+	}
+
+	private setPaginationCount(count: number): void {
+		this.gridModel.pagination({ count });


not sure that we need a dedicated method to just setup pagination count

klumba12 · 2022-03-24T13:31:31Z

packages/qgrid-ngx-plugins/src/lib/data-provider/data-provider-strategy.ts

@@ -0,0 +1,13 @@
+import { GridModel } from '@qgrid/ngx';


can u put all strategies directly to the data-provider folder? also remove please index.ts file, we don't use them

klumba12 · 2022-03-24T13:31:50Z

packages/qgrid-ngx-plugins/src/lib/data-provider/models.ts

@@ -0,0 +1,13 @@
+import { GridModel } from '@qgrid/ngx';


rename file to the server.ts or maybe page-server.ts and PageServer class? not sure

Renamed interface to DataProviderPageServer

klumba12 · 2022-03-24T13:33:02Z

packages/qgrid-ngx-examples/src/examples/data-provider/example-data-provider.component.ts

@@ -21,23 +23,29 @@ export class ExampleDataProviderComponent {

  page$: Observable<Atom[]>;

+	dataProvider: DataProvider<Atom>;


can we again try to initialize dataProvider right here?

klumba12 · 2022-03-25T11:05:09Z

packages/qgrid-ngx-plugins/src/lib/data-provider/cache-requested-pages.ts

@@ -0,0 +1,45 @@
+import { GridModel } from '@qgrid/ngx';


file name should be cache-already-requested-page-strategy.ts

klumba12 · 2022-03-25T11:06:31Z

packages/qgrid-ngx-plugins/src/lib/data-provider/cache-requested-pages.ts

+
+	constructor(
+		private server: Pick<DataProviderPageServer<T>, 'getPage'>,
+		private pagesToLoad: number = 0,


it should be options { pagesToLoad: number }, consider to rename pagesToLoad to numberOfBackgroundPages or backgroundPages

klumba12 · 2022-03-25T11:07:43Z

packages/qgrid-ngx-plugins/src/lib/data-provider/cache-requested-pages.ts

+		const { current, size } = model.pagination();
+
+		if (this.pagesToLoad) {
+			this.loadInBackground(this.pagesToLoad, model);


what if process will conflict with loadInBackground? need to have synchronization mechanizm

klumba12 · 2022-03-25T11:08:02Z

packages/qgrid-ngx-plugins/src/lib/data-provider/cache-requested-pages.ts

+		private pagesToLoad: number = 0,
+	) { }
+
+	process(memo: T[], { model }: DataProviderProcessContext): Observable<T[]> {


rename memo to data

klumba12 · 2022-03-25T11:08:53Z

packages/qgrid-ngx-plugins/src/lib/data-provider/cache-requested-pages.ts

+			return of(this.gridRowsCache.get(current));
+		}
+
+		const shouldRequestData = !memo?.length;


why memo can be undefined? our method signature doesn't accept it

yeah. It's always at least an empty array. Removed question mark

klumba12 · 2022-03-25T11:09:47Z

packages/qgrid-ngx-plugins/src/lib/data-provider/data-provider-page-server.ts

+import { Observable } from 'rxjs';
+
+export interface DataProviderPageServer<T> {
+	getPage(page?: number, size?: number): Observable<T[]>;


why page and size are optional?

made them required

klumba12 · 2022-03-25T11:10:21Z

packages/qgrid-ngx-plugins/src/lib/data-provider/data-provider-process-context.ts

@@ -0,0 +1,5 @@
+import { GridModel } from '@qgrid/ngx';
+
+export interface DataProviderProcessContext {


can you rename it to DataProviderContext and put under the data-provider.ts file?

klumba12 · 2022-03-25T11:10:34Z

packages/qgrid-ngx-plugins/src/lib/data-provider/data-provider-strategy.ts

+import { DataProviderProcessContext } from './data-provider-process-context';
+
+export interface DataProviderStrategy<T> {
+	process(memo: T[], context: DataProviderProcessContext): Observable<T[]>;


memo -> data

klumba12 · 2022-03-25T11:11:15Z

packages/qgrid-ngx-plugins/src/lib/data-provider/request-count-once.ts

+		private server: Pick<DataProviderPageServer<T>, 'getTotal'>,
+	) { }
+
+	process(memo: T[], { model }: DataProviderProcessContext): Observable<T[]> {


memo -> data

klumba12 · 2022-03-25T11:11:50Z

packages/qgrid-ngx-plugins/src/lib/data-provider/request-count-once.ts

+	) { }
+
+	process(memo: T[], { model }: DataProviderProcessContext): Observable<T[]> {
+		if (this.totalCount > 0) {


what will happen if count will be 0 from the server?

if there was empty array from server wi will try to send that request everytime. Do you think we should remove it?

klumba12 · 2022-03-30T12:15:27Z

packages/qgrid-ngx-plugins/src/lib/data-provider/cache-already-requested-page-strategy.ts

+
+	constructor(
+		private server: Pick<DataProviderPageServer<T>, 'getPage'>,
+		private options: { pagesToLoad: number } = { pagesToLoad: 0 },


by default pagesToLoad should be equal to 1, if more than 1 do background load

klumba12 · 2022-03-30T12:16:08Z

packages/qgrid-ngx-plugins/src/lib/data-provider/cache-already-requested-page-strategy.ts

+	process(data: T[], { model }: DataProviderContext): Observable<T[]> {
+		const { current, size } = model.pagination();
+
+		if (this.paginationSize !== size) {


move it under the separate method like invalidateCache() will be more readable

klumba12 · 2022-03-30T12:17:14Z

packages/qgrid-ngx-plugins/src/lib/data-provider/cache-already-requested-page-strategy.ts

+		}
+
+		const shouldRequestData = !data.length;
+		return (shouldRequestData ? this.server.getPage(current, size) : of(data))


with if statement will be more readable

klumba12 · 2022-03-30T12:18:01Z

packages/qgrid-ngx-plugins/src/lib/data-provider/cache-already-requested-page-strategy.ts

+			.pipe(tap(rows => this.pageCache.set(current, rows)));
+	}
+
+	private loadInBackground(pagesToLoad: number, model: GridModel): void {


loadBackgroundPages maybe beters?

Dalantren added 4 commits March 22, 2022 18:27

Added DataProvider Strategy class and interfaces

b44d38c

Added basic data provider strategies

abdd742

Changed public API

5896dad

Updated example with data-provider strategies

1366258

klumba12 reviewed Mar 22, 2022

View reviewed changes

Dalantren added 2 commits March 23, 2022 15:15

Fixed comments

2d6513e

Added loading page in background for cache strategy

7e88f63

klumba12 reviewed Mar 24, 2022

View reviewed changes

Fixed comments

862926b

klumba12 reviewed Mar 25, 2022

View reviewed changes

Dalantren added 3 commits March 28, 2022 15:39

Fixed comments

8e435ac

Added cache reset on paginationSize changes

5c848a8

Moved data provider initialization

5410eca

klumba12 reviewed Mar 30, 2022

View reviewed changes

Dalantren added 6 commits April 4, 2022 13:32

Fixed comments

4b77842

Merge branch 'master' into feature/data-provider-strategies

99e1bb8

Added Check next page strategy

ac56e85

Moved invalidation strategy into data provider.

ab5c721

Merge branch 'master' into feature/data-provider-strategies

ca9c107

Updated getPage to getRecords for DataProviderServer

5e06bc4

		@@ -21,23 +23,29 @@ export class ExampleDataProviderComponent {

		page$: Observable<Atom[]>;

		dataProvider: DataProvider<Atom>;

		@@ -0,0 +1,5 @@
		import { GridModel } from '@qgrid/ngx';

		export interface DataProviderProcessContext {

Feature/data provider strategies #585

Are you sure you want to change the base?

Feature/data provider strategies #585

Conversation

Dalantren commented Mar 22, 2022

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klumba12 Mar 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dalantren Mar 25, 2022 • 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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klumba12 Mar 25, 2022 • 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

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

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

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

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

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

klumba12 Mar 24, 2022 •

edited

Loading

Dalantren Mar 25, 2022 •

edited

Loading

klumba12 Mar 25, 2022 •

edited

Loading