Merge pull request #2834 from bcameron1231/v3-2730revert

Revert Accidental Async Iterator update from V4

CHANGELOG.md

@@ -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.

debug/launch/graph.ts

@@ -17,4 +17,4 @@ export async function Example(settings: any) {
     });
 
     process.exit(0);
-}
+}

docs/graph/onedrive.md

@@ -577,16 +577,6 @@ const delta: IDeltaItems = await graph.users.getById("user@tenant.onmicrosoft.co
 
 // 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

packages/graph/behaviors/paged.ts

@@ -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) => {
 
@@ -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),
             };
 

packages/graph/directory-objects/types.ts

@@ -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
@@ -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> { }

packages/graph/graphqueryable.ts

@@ -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];
 
@@ -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> { }

packages/graph/onedrive/types.ts

@@ -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
@@ -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,
             };
@@ -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 { }

packages/graph/onedrive/users.ts

@@ -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));

test/graph/paging.ts

@@ -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 () {
@@ -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);
     });
 });