Skip to content

Commit

Permalink
Revert Accidental Async Iterator update from V4
Browse files Browse the repository at this point in the history
  • Loading branch information
bcameron1231 committed Nov 20, 2023
1 parent 1b3b6fa commit 7c96ba5
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 93 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -567,4 +567,4 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Dropped entire package, no longer needed

- config-store:
- Dropped entire package.
- Dropped entire package.
2 changes: 1 addition & 1 deletion debug/launch/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ export async function Example(settings: any) {
});

process.exit(0);
}
}
10 changes: 0 additions & 10 deletions docs/graph/onedrive.md
Original file line number Diff line number Diff line change
Expand Up @@ -577,16 +577,6 @@ const delta: IDeltaItems = await graph.users.getById("[email protected]

// Get the changes for the drive items from token
const delta: IDeltaItems = await graph.me.drive.root.delta("{token}")();

// consume all the values using async iterator
for await (const val of delta.next.paged()) {
console.log(JSON.stringify(val, null, 2));
}

// consume all the values using async iterator in pages of 20
for await (const val of delta.next.top(20).paged()) {
console.log(JSON.stringify(val, null, 2));
}
```

## Get Drive Item Analytics
Expand Down
71 changes: 20 additions & 51 deletions packages/graph/behaviors/paged.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,81 +5,50 @@ import { ConsistencyLevel } from "./consistency-level.js";

export interface IPagedResult {
count: number;
value: any | any[] | null;
value: any[] | null;
hasNext: boolean;
nextLink: string;
next(): Promise<IPagedResult>;
}

/**
* A function that will take a collection defining IGraphQueryableCollection and return the count of items
* in that collection. Not all Graph collections support Count.
*
* @param col The collection to count
* @returns number representing the count
*/
export async function Count<T>(col: IGraphQueryableCollection<T>): Promise<number> {

const q = GraphQueryableCollection(col).using(Paged(), ConsistencyLevel());
q.query.set("$count", "true");
q.top(1);

const y: IPagedResult = await q();
return y.count;
}

/**
* Configures a collection query to returned paged results via async iteration
* Configures a collection query to returned paged results
*
* @param col Collection forming the basis of the paged collection, this param is NOT modified
* @returns A duplicate collection which will return paged results
*/
export function AsAsyncIterable<T>(col: IGraphQueryableCollection<T>): AsyncIterable<T> {
export function AsPaged(col: IGraphQueryableCollection, supportsCount = false): IGraphQueryableCollection {

const q = GraphQueryableCollection(col).using(Paged(), ConsistencyLevel());
const q = GraphQueryableCollection(col).using(Paged(supportsCount), ConsistencyLevel());

const queryParams = ["$search", "$top", "$select", "$expand", "$filter", "$orderby"];

if (supportsCount) {

// we might be constructing our query with a next url that will already contain $count so we need
// to ensure we don't add it again, likewise if it is already in our query collection we don't add it again
if (!q.query.has("$count") && !/\$count=true/i.test(q.toUrl())) {
q.query.set("$count", "true");
}

queryParams.push("$count");
}

for (let i = 0; i < queryParams.length; i++) {
const param = col.query.get(queryParams[i]);
if (objectDefinedNotNull(param)) {
q.query.set(queryParams[i], param);
}
}

return {

[Symbol.asyncIterator]() {
return <AsyncIterator<T>>{

_next: q,

async next() {

if (this._next === null) {
return { done: true, value: undefined };
}

const result: IPagedResult = await this._next();

if (result.hasNext) {
this._next = GraphQueryableCollection([this._next, result.nextLink]);
return { done: false, value: result.value };
} else {
this._next = null;
return { done: false, value: result.value };
}
},
};
},
};
return q;
}

/**
* Behavior that converts results to pages when used with a collection (exposed through the paged method of GraphCollection)
*
* @returns A TimelinePipe used to configure the queryable
*/
export function Paged(): TimelinePipe {
export function Paged(supportsCount = false): TimelinePipe {

return (instance: IGraphQueryable) => {

Expand All @@ -90,14 +59,14 @@ export function Paged(): TimelinePipe {
const json = txt.replace(/\s/ig, "").length > 0 ? JSON.parse(txt) : {};
const nextLink = json["@odata.nextLink"];

const count = hOP(json, "@odata.count") ? parseInt(json["@odata.count"], 10) : -1;
const count = supportsCount && hOP(json, "@odata.count") ? parseInt(json["@odata.count"], 10) : 0;

const hasNext = !stringIsNullOrEmpty(nextLink);

result = {
count,
hasNext,
nextLink: hasNext ? nextLink : null,
next: () => (hasNext ? AsPaged(GraphQueryableCollection([instance, nextLink]), supportsCount)() : null),
value: parseODataJSON(json),
};

Expand Down
15 changes: 13 additions & 2 deletions packages/graph/directory-objects/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { DirectoryObject as IDirectoryObjectType } from "@microsoft/microsoft-gr
import { defaultPath, getById, IGetById, deleteable, IDeleteable } from "../decorators.js";
import { body } from "@pnp/queryable";
import { graphPost } from "../operations.js";
import { Count } from "../behaviors/paged.js";
import { AsPaged, IPagedResult } from "../behaviors/paged.js";

/**
* Represents a Directory Object entity
Expand Down Expand Up @@ -65,7 +65,18 @@ export class _DirectoryObjects<GetType = IDirectoryObjectType[]> extends _GraphQ
* If the resource doesn't support count, this value will always be zero
*/
public async count(): Promise<number> {
return Count(this);
const q = AsPaged(this, true);
const r: IPagedResult = await q.top(1)();
return r.count;
}

/**
* Allows reading through a collection as pages of information whose size is determined by top or the api method's default
*
* @returns an object containing results, the ability to determine if there are more results, and request the next page of results
*/
public paged(): Promise<IPagedResult> {
return AsPaged(this, true)();
}
}
export interface IDirectoryObjects extends _DirectoryObjects, IGetById<IDirectoryObjectType> { }
Expand Down
11 changes: 6 additions & 5 deletions packages/graph/graphqueryable.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { isArray } from "@pnp/core";
import { IInvokable, Queryable, queryableFactory } from "@pnp/queryable";
import { ConsistencyLevel } from "./behaviors/consistency-level.js";
import { AsAsyncIterable } from "./behaviors/paged.js";
import { AsPaged, IPagedResult } from "./behaviors/paged.js";

export type GraphInit = string | IGraphQueryable | [IGraphQueryable, string];

Expand Down Expand Up @@ -167,17 +167,18 @@ export class _GraphQueryableCollection<GetType = any[]> extends _GraphQueryable<
* If the resource doesn't support count, this value will always be zero
*/
public async count(): Promise<number> {
// TODO::do we want to do this, or just attach count to the collections that support it? we could use a decorator for countable on the few collections that support count.
return -1;
const q = AsPaged(this);
const r: IPagedResult = await q.top(1)();
return r.count;
}

/**
* Allows reading through a collection as pages of information whose size is determined by top or the api method's default
*
* @returns an object containing results, the ability to determine if there are more results, and request the next page of results
*/
public paged(): AsyncIterable<GetType> {
return AsAsyncIterable(this);
public paged(): Promise<IPagedResult> {
return AsPaged(this)();
}
}
export interface IGraphQueryableCollection<GetType = any[]> extends _GraphQueryableCollection<GetType> { }
Expand Down
11 changes: 5 additions & 6 deletions packages/graph/onedrive/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { defaultPath, getById, IGetById, deleteable, IDeleteable, updateable, IU
import { body, BlobParse, CacheNever, errorCheck, InjectHeaders } from "@pnp/queryable";
import { graphPatch, graphPost, graphPut } from "../operations.js";
import { driveItemUpload } from "./funcs.js";
import { AsPaged } from "../behaviors/paged.js";

/**
* Describes a Drive instance
Expand Down Expand Up @@ -149,17 +150,15 @@ export class _Root extends _GraphQueryableInstance<IDriveItemType> {
public delta(token?: string): IGraphQueryableCollection<IDeltaItems> {
const path = `delta${(token) ? `(token=${token})` : ""}`;

const query: IGraphQueryableCollection<IDeltaItems> = <any>GraphQueryableCollection(this, path);
const query = GraphQueryableCollection(this, path);
query.on.parse.replace(errorCheck);
query.on.parse(async (url: URL, response: Response, result: any): Promise<[URL, Response, any]> => {

const json = await response.json();
const nextLink = json["@odata.nextLink"];
const deltaLink = json["@odata.deltaLink"];

result = {
// TODO:: update docs to show how to load next with async iterator
next: () => (nextLink ? GraphQueryableCollection([this, nextLink]) : null),
next: () => (nextLink ? AsPaged(GraphQueryableCollection([this, nextLink]))() : null),
delta: () => (deltaLink ? GraphQueryableCollection([query, deltaLink])() : null),
values: json.value,
};
Expand Down Expand Up @@ -351,8 +350,8 @@ export class _DriveItem extends _GraphQueryableInstance<IDriveItemType> {
* @returns IGraphQueryableCollection<IItemAnalytics>
*/
public analytics(analyticsOptions?: IAnalyticsOptions): IGraphQueryableCollection<IItemAnalytics> {
const query = `analytics/${analyticsOptions ? analyticsOptions.timeRange : "lastSevenDays"}`;
return <IGraphQueryableCollection<IItemAnalytics>>GraphQueryableCollection(this, query);
const query = `analytics/${analyticsOptions?analyticsOptions.timeRange:"lastSevenDays"}`;
return GraphQueryableCollection(this, query);
}
}
export interface IDriveItem extends _DriveItem, IDeleteable, IUpdateable { }
Expand Down
8 changes: 7 additions & 1 deletion packages/graph/onedrive/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ _Drive.prototype.special = function special(specialFolder: SpecialFolder): IDriv
return DriveItem(this, `special/${specialFolder}`);
};

export type SpecialFolder = "documents" | "photos" | "cameraroll" | "approot" | "music";
export enum SpecialFolder {
"Documents" = "documents",
"Photos" = "photos",
"CameraRoll" = "cameraroll",
"AppRoot" = "approot",
"Music" = "music",
}

_DriveItem.prototype.restore = function restore(restoreOptions: IItemOptions): Promise<IDriveItem> {
return graphPost(DriveItem(this, "restore"), body(restoreOptions));
Expand Down
71 changes: 55 additions & 16 deletions test/graph/paging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,32 +35,48 @@ describe("Groups", function () {
}
});

it("pages users 1", async function () {

let users = await this.pnp.graph.users.top(2).paged();

expect(users).to.have.property("hasNext", true);

users = await users.next();

expect(users).to.have.property("hasNext", true);
});

it("pages all users", async function () {

const count = await this.pnp.graph.users.count();

const allUsers = [];
let users = await this.pnp.graph.users.top(20).select("displayName").paged();

for await (const users of this.pnp.graph.users.top(20).select("displayName").paged()) {
allUsers.push(...users);
allUsers.push(...users.value);

while (users.hasNext) {
users = await users.next();
allUsers.push(...users.value);
}

expect(allUsers.length).to.eq(count);
});

it("pages groups", async function () {

const count = await this.pnp.graph.groups.count();

expect(count).is.gt(0);
let groups = await this.pnp.graph.groups.top(2).paged();

const allGroups = [];
expect(groups).to.have.property("hasNext", true);
expect(groups).to.have.property("count").gt(0);
expect(groups.value.length).to.eq(2);

for await (const groups of this.pnp.graph.groups.top(20).select("displayName").paged()) {
allGroups.push(...groups);
}
groups = await groups.next();

expect(allGroups.length).to.eq(count);
expect(groups).to.have.property("hasNext", true);
// count only returns on the first call, not subsequent paged calls
expect(groups).to.have.property("count").eq(0);
expect(groups.value.length).to.eq(2);
});

it("groups count", async function () {
Expand All @@ -70,22 +86,45 @@ describe("Groups", function () {
expect(count).to.be.gt(0);
});

it("pages items", async function () {
it("pages all groups", async function () {

const allItems = [];
const count = await this.pnp.graph.groups.count();

const allGroups = [];
let groups = await this.pnp.graph.groups.top(20).select("mailNickname").paged();

for await (const items of itemsCol.paged()) {
allItems.push(...items);
allGroups.push(...groups.value);

while (groups.hasNext) {
groups = await groups.next();
allGroups.push(...groups.value);
}

expect(allItems.length).to.be.gt(0);
expect(allGroups.length).to.be.gt((count - 10)).and.lt((count + 10));
});

it("pages items", async function () {

let pagedResults = await itemsCol.top(5).paged();

expect(pagedResults.value.length).to.eq(5);
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
expect(pagedResults.hasNext).to.be.true;
expect(pagedResults.count).to.eq(0);

pagedResults = await pagedResults.next();

expect(pagedResults.value.length).to.eq(5);
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
expect(pagedResults.hasNext).to.be.true;
expect(pagedResults.count).to.eq(0);
});

it("items count", async function () {

const count = await itemsCol.count();

// items doesn't support count, should be zero
expect(count).to.eq(-1);
expect(count).to.eq(0);
});
});

0 comments on commit 7c96ba5

Please sign in to comment.