app catalog fixes, test-recorder initial work

.gitignore

@@ -18,6 +18,8 @@ coverage
 # node-waf configuration
 .lock-wscript
 
+.test-recording
+
 # Dependency directory
 node_modules
 

debug/launch/v3-patrick.ts

@@ -7,6 +7,10 @@ import "@pnp/sp/lists";
 import "@pnp/sp/files";
 import "@pnp/sp/folders";
 import "@pnp/sp/appcatalog";
+import { Web } from "@pnp/sp/webs";
+import { AssignFrom } from "@pnp/core";
+import { RequestRecorderCache } from "../../test/test-recorder.js";
+import { join } from "path";
 
 declare var process: { exit(code?: number): void };
 
@@ -43,16 +47,20 @@ export async function Example(settings: ITestingSettings) {
 
     try {
 
+        const recordingPath = join("C:/github/@pnp-fork", ".test-recording");
+
         const sp2 = spfi("https://318studios.sharepoint.com/sites/dev").using(SPDefault({
             msal: {
                 config: settings.testing.sp.msal.init,
                 scopes: settings.testing.sp.msal.scopes,
             },
-        })).using(PnPLogging(LogLevel.Verbose));
+        })).using(PnPLogging(LogLevel.Verbose)).using(RequestRecorderCache(recordingPath, "record", () => false));
 
         const web = await sp2.getTenantAppCatalogWeb();
 
-        const y = await web();
+        const web2 = Web("https://318studios.sharepoint.com/sites/dev").using(AssignFrom(web));
+
+        const y = await web2();
 
         console.log(JSON.stringify(y));
 
@@ -61,8 +69,6 @@ export async function Example(settings: ITestingSettings) {
         console.error(e);
     }
 
-    console.log("here");
-
     // const [batchedSP, execute] = sp2.batched();
 
     // let res = [];

package.json

@@ -39,6 +39,7 @@
         "node-abort-controller": "^3.0.1",
         "node-fetch": "^2.6.1",
         "prettyjson": "^1.2.1",
+        "stacktrace-js": "^2.0.2",
         "string-replace-loader": "^3.0.1",
         "tslib": "^2.1.0",
         "typescript": "^4.2.3",

packages/core/timeline.ts

@@ -80,7 +80,7 @@ export abstract class Timeline<T extends Moments> {
 
     private _onProxy: typeof Proxy | null = null;
     private _emitProxy: typeof Proxy | null = null;
-    private _inheritingObservers: boolean;
+    protected _inheritingObservers: boolean;
 
     constructor(protected readonly moments: T, protected observers: ObserverCollection = {}) {
         this._inheritingObservers = true;

packages/sp/appcatalog/index.ts

@@ -18,16 +18,10 @@ declare module "../fi" {
 
 SPFI.prototype.getTenantAppCatalogWeb = async function (this: SPFI): Promise<IWeb> {
 
-
     return this.create(async (q) => {
 
-        const data: { CorporateCatalogUrl: string } = await Web(q.toUrl().replace(/\/_api\/.*$/i, ""), "/_api/SP_TenantSettings_Current").using(AssignFrom(q))();
-
-        console.log(data);
+        const data = await Web(q.toUrl().replace(/\/_api\/.*$/i, ""), "/_api/SP_TenantSettings_Current").using(AssignFrom(q))<{ CorporateCatalogUrl: string }>();
 
         return Web(data.CorporateCatalogUrl).using(AssignFrom(q));
-
     });
-
-    return null;
 };

packages/sp/appcatalog/web.ts

@@ -1,21 +1,17 @@
+import { addProp } from "@pnp/queryable/add-prop.js";
 import { _Web } from "../webs/types.js";
 import { AppCatalog, IAppCatalog } from "./types.js";
 
 declare module "../webs/types" {
     interface _Web {
-        getAppCatalog(url?: string | _Web): IAppCatalog;
+        appcatalog: IAppCatalog;
     }
     interface IWeb {
         /**
-         * Gets this web (default) or the web specifed by the optional string case
-         * as an IAppCatalog instance
-         *
-         * @param url [Optional] Url of the web to get (default: current web)
+         * Gets the appcatalog (if it exists associated with this web)
          */
-        getAppCatalog(url?: string | _Web): IAppCatalog;
+        appcatalog: IAppCatalog;
     }
 }
 
-_Web.prototype.getAppCatalog = function (this: _Web, url?: string | _Web): IAppCatalog {
-    return AppCatalog(url || this);
-};
+addProp(_Web, "appcatalog", AppCatalog);

packages/sp/batching.ts

@@ -1,4 +1,4 @@
-import { getGUID, isUrlAbsolute, combine, CopyFrom, TimelinePipe } from "@pnp/core";
+import { getGUID, isUrlAbsolute, combine, CopyFrom, TimelinePipe, isFunc } from "@pnp/core";
 import { InjectHeaders, IQueryableInternal, parseBinderWithErrorCheck, Queryable } from "@pnp/queryable";
 import { spPost } from "./operations";
 import { _SPQueryable } from "./spqueryable";
@@ -232,12 +232,17 @@ export function createBatch(base: IQueryableInternal): [TimelinePipe, () => Prom
         // we need to know when each request in the batch's timeline has completed
         instance.on.dispose(function () {
 
-            // let things know we are done with this request
-            (<any>this)[RequestCompleteSym]();
+            if (isFunc((<any>this)[RequestCompleteSym])) {
 
-            // remove the symbol props we added for good hygene
-            delete this[RegistrationCompleteSym];
-            delete this[RequestCompleteSym];
+                // let things know we are done with this request
+                (<any>this)[RequestCompleteSym]();
+                delete this[RequestCompleteSym];
+            }
+
+            if (isFunc((<any>this)[RegistrationCompleteSym])) {
+                // remove the symbol props we added for good hygene
+                delete this[RegistrationCompleteSym];
+            }
         });
 
         return instance;

test/main.ts

@@ -12,6 +12,8 @@ import "@pnp/sp/webs";
 import { IWeb, IWebInfo } from "@pnp/sp/webs";
 import { graphfi, GraphFI } from "@pnp/graph";
 import { LogLevel } from "@pnp/logging";
+import { RequestRecorderCache } from "./test-recorder.js";
+import { join } from "path";
 
 chai.use(chaiAsPromised);
 
@@ -198,7 +200,7 @@ async function spTestSetup(ts: ISettings): Promise<void> {
             config: settings.testing.sp.msal.init,
             scopes: settings.testing.sp.msal.scopes,
         },
-    })).using(TestLogging());
+    })).using(TestLogging()); // .using(RequestRecorderCache(join("C:/github/@pnp-fork", ".test-recording"), "record", () => false));
 }
 
 async function graphTestSetup(): Promise<void> {
@@ -207,7 +209,7 @@ async function graphTestSetup(): Promise<void> {
             config: settings.testing.graph.msal.init,
             scopes: settings.testing.graph.msal.scopes,
         },
-    })).using(TestLogging());
+    })).using(TestLogging()); // .using(RequestRecorderCache(join("C:/github/@pnp-fork", ".test-recording"), "record", () => false));
 }
 
 export const testSettings: ISettings = settings.testing;

test/sp/appcatalog.ts

@@ -1,5 +1,5 @@
 
-import { getRandomString } from "@pnp/core";
+import { getRandomString, delay } from "@pnp/core";
 import { expect } from "chai";
 import { getSP, testSettings } from "../main.js";
 import { IAppCatalog } from "@pnp/sp/appcatalog";
@@ -11,10 +11,6 @@ import * as path from "path";
 import findupSync = require("findup-sync");
 import { SPFI } from "@pnp/sp";
 
-const sleep = (ms: number) => new Promise<void>(r => setTimeout(() => {
-    r();
-}, ms));
-
 // give ourselves a single reference to the projectRoot
 const projectRoot = path.resolve(path.dirname(findupSync("package.json")));
 
@@ -32,7 +28,7 @@ describe.skip("AppCatalog", function () {
         before(function () {
             _spfi = getSP();
             // appCatWeb = await sp.getTenantAppCatalogWeb();
-            appCatalog = _spfi.web.getAppCatalog();
+            appCatalog = _spfi.web.appcatalog;
             // return Promise.resolve();
         });
 
@@ -64,22 +60,22 @@ describe.skip("AppCatalog", function () {
         });
 
         it("it installs an app on a web", async function () {
-            const myApp = _spfi.web.getAppCatalog().getAppById(appId);
+            const myApp = _spfi.web.appcatalog.getAppById(appId);
             return expect(myApp.install(), `app '${appId}' should've been installed on web ${testSettings.sp.webUrl}`).to.eventually.be.fulfilled;
         });
 
         it("it uninstalls an app", async function () {
             // We have to make sure the app is installed before we can uninstall it otherwise we get the following error message:
             // Another job exists for this app instance. Please retry after that job is done.
-            const myApp = _spfi.web.getAppCatalog().getAppById(appId);
+            const myApp = _spfi.web.appcatalog.getAppById(appId);
             let app = { InstalledVersion: "" };
             let retryCount = 0;
 
             do {
                 if (retryCount === 5) {
                     break;
                 }
-                await sleep(10000); // Sleep for 10 seconds
+                await delay(10000); // Sleep for 10 seconds
                 app = await myApp();
                 retryCount++;
             } while (app.InstalledVersion === "");
@@ -88,7 +84,7 @@ describe.skip("AppCatalog", function () {
         });
 
         it("it upgrades an app", async function () {
-            const myApp = _spfi.web.getAppCatalog().getAppById(appId);
+            const myApp = _spfi.web.appcatalog.getAppById(appId);
             return expect(myApp.upgrade(), `app '${appId}' should've been upgraded on web ${testSettings.sp.webUrl}`).to.eventually.be.fulfilled;
         });
 

test/test-recorder.ts

@@ -0,0 +1,58 @@
+import { isFunc, TimelinePipe, dateAdd, getHashCode } from "@pnp/core";
+import { Queryable } from "@pnp/queryable";
+import { statSync, readFileSync, existsSync, writeFileSync, mkdirSync } from "fs";
+import { join, basename } from "path";
+import { getSync } from "stacktrace-js";
+
+export function RequestRecorderCache(resolvedRecordingPath: string, mode: "readonly" | "record", isExpired?: (Date) => boolean): TimelinePipe {
+
+    const today = new Date();
+    const _isExpired = isFunc(isExpired) ? isExpired : (d: Date) => dateAdd(d, "week", 2) < today;
+    const recorderFileKey = Symbol.for("recorder_file_key");
+    const recorderFilePath = Symbol.for("recorder_file_path");
+
+    if (!existsSync(resolvedRecordingPath)) {
+        mkdirSync(resolvedRecordingPath);
+    }
+
+    return (instance: Queryable) => {
+
+        instance.on.pre(async function (this: Queryable, url: string, init: RequestInit, result: any): Promise<[string, RequestInit, any]> {
+
+            const stack = getSync();
+
+            this[recorderFileKey] = getHashCode(`${init.method}:${url}:${basename(stack[0].fileName)}:${stack[0].lineNumber}:${stack[0].columnNumber}`).toString();
+            this[recorderFilePath] = join(resolvedRecordingPath, `result.${this[recorderFileKey]}.json`);
+
+            if (existsSync(this[recorderFilePath])) {
+
+                const stats = statSync(this[recorderFilePath]);
+                if (!_isExpired(stats.mtime)) {
+                    result = JSON.parse(readFileSync(this[recorderFilePath]).toString());
+                    return [url, init, result];
+                }
+            }
+
+            if (mode === "record") {
+
+                this.on.post(async function (url: URL, result: any) {
+
+                    if (Reflect.has(this, recorderFilePath)) {
+                        writeFileSync(this[recorderFilePath], JSON.stringify(result));
+                    }
+
+                    return [url, result];
+                });
+            }
+
+            return [url, init, result];
+        });
+
+        instance.on.dispose(function () {
+            delete this[recorderFileKey];
+            delete this[recorderFilePath];
+        });
+
+        return instance;
+    };
+}

~status.md

@@ -3,6 +3,7 @@
 // TODO:: do we want to move to .env files, seems to be a sorta "norm" folks are using?
 // TODO:: maintain an experimental release
 // TODO:: need to update our /samples and maybe more? remove rollup sample
+// TODO:: kebab-case all file names
 
 ## experiments: