Refactor caching system

Author: Ndpnt

src/reporter/github.js

@@ -30,7 +30,22 @@ export default class GitHub {
     this.commonParams = { owner, repo };
 
     this.issuesCache = new Map();
-    this.cacheLoadedPromise = null;
+    this._issuesPromise = null;
+  }
+
+  get issues() {
+    if (!this._issuesPromise) {
+      logger.info('Loading issues from GitHub…');
+      this._issuesPromise = this.loadAllIssues();
+    }
+
+    return this._issuesPromise;
+  }
+
+  clearCache() {
+    this.issuesCache.clear();
+    this._issuesPromise = null;
+    logger.info('Issues cache cleared');
   }
 
   async initialize() {
@@ -74,29 +89,13 @@ export default class GitHub {
         }
       });
 
-      logger.info(`Cached ${onlyIssues.length} issues from the repository`);
-    } catch (error) {
-      logger.error(`Failed to load issues: ${error.message}`);
-    }
-  }
+      logger.info(`Cached ${onlyIssues.length} issues from the GitHub repository`);
 
-  async refreshIssuesCache() {
-    try {
-      logger.info('Refreshing issues cache from GitHub…');
-      this.issuesCache.clear();
-      this.cacheLoadedPromise = this.loadAllIssues();
-      await this.cacheLoadedPromise;
-      logger.info('Issues cache refreshed successfully');
+      return this.issuesCache;
     } catch (error) {
-      logger.error(`Failed to refresh issues cache: ${error.message}`);
-    }
-  }
-
-  async ensureCacheLoaded() {
-    if (!this.cacheLoadedPromise) {
-      this.cacheLoadedPromise = this.loadAllIssues();
+      logger.error(`Failed to load issues: ${error.message}`);
+      throw error;
     }
-    await this.cacheLoadedPromise;
   }
 
   async getRepositoryLabels() {
@@ -114,9 +113,11 @@ export default class GitHub {
     });
   }
 
-  async createIssue({ title, description: body, labels }) {
-    await this.ensureCacheLoaded();
+  async getIssue(title) {
+    return (await this.issues).get(title);
+  }
 
+  async createIssue({ title, description: body, labels }) {
     const { data: issue } = await this.octokit.request('POST /repos/{owner}/{repo}/issues', {
       ...this.commonParams,
       title,
@@ -130,8 +131,6 @@ export default class GitHub {
   }
 
   async updateIssue(issue, { state, labels }) {
-    await this.ensureCacheLoaded();
-
     const { data: updatedIssue } = await this.octokit.request('PATCH /repos/{owner}/{repo}/issues/{issue_number}', {
       ...this.commonParams,
       issue_number: issue.number,
@@ -144,13 +143,7 @@ export default class GitHub {
     return updatedIssue;
   }
 
-  getIssue(title) {
-    return this.issuesCache.get(title) || null;
-  }
-
   async addCommentToIssue({ issue, comment: body }) {
-    await this.ensureCacheLoaded();
-
     const { data: comment } = await this.octokit.request('POST /repos/{owner}/{repo}/issues/{issue_number}/comments', {
       ...this.commonParams,
       issue_number: issue.number,
@@ -162,9 +155,7 @@ export default class GitHub {
 
   async closeIssueWithCommentIfExists({ title, comment }) {
     try {
-      await this.ensureCacheLoaded();
-
-      const issue = this.getIssue(title);
+      const issue = await this.getIssue(title);
 
       if (!issue || issue.state == GitHub.ISSUE_STATE_CLOSED) {
         return;
@@ -183,9 +174,7 @@ export default class GitHub {
 
   async createOrUpdateIssue({ title, description, label }) {
     try {
-      await this.ensureCacheLoaded();
-
-      const issue = this.getIssue(title);
+      const issue = await this.getIssue(title);
 
       if (!issue) {
         const createdIssue = await this.createIssue({ title, description, labels: [label] });
@@ -194,7 +183,6 @@ export default class GitHub {
       }
 
       const managedLabelsNames = this.MANAGED_LABELS.map(label => label.name);
-
       const labelsNotManagedToKeep = issue.labels.map(label => label.name).filter(label => !managedLabelsNames.includes(label));
       const [managedLabel] = issue.labels.filter(label => managedLabelsNames.includes(label.name)); // It is assumed that only one specific reason for failure is possible at a time, making managed labels mutually exclusive
 

src/reporter/github.test.js

@@ -22,7 +22,7 @@ describe('GitHub', function () {
       .get('/repos/owner/repo/issues')
       .query(true)
       .reply(200, [ EXISTING_OPEN_ISSUE, EXISTING_CLOSED_ISSUE ]);
-    await github.refreshIssuesCache();
+    await github.clearCache();
   });
 
   describe('#initialize', () => {
@@ -143,15 +143,22 @@ describe('GitHub', function () {
   });
 
   describe('#getIssue', () => {
+    before(() => {
+      nock('https://api.github.com')
+        .get('/repos/owner/repo/issues')
+        .query(true)
+        .reply(200, [ EXISTING_OPEN_ISSUE, EXISTING_CLOSED_ISSUE ]);
+    });
+
     context('when the issue exists in the cache', () => {
-      it('returns the cached issue', () => {
-        expect(github.getIssue(EXISTING_OPEN_ISSUE.title)).to.deep.equal(EXISTING_OPEN_ISSUE);
+      it('returns the cached issue', async () => {
+        expect(await github.getIssue(EXISTING_OPEN_ISSUE.title)).to.deep.equal(EXISTING_OPEN_ISSUE);
       });
     });
 
     context('when the issue does not exist in the cache', () => {
-      it('returns null', () => {
-        expect(github.getIssue('Non-existent Issue')).to.be.null;
+      it('returns undefined', async () => {
+        expect(await github.getIssue('Non-existent Issue')).to.be.undefined;
       });
     });
   });
@@ -264,7 +271,7 @@ describe('GitHub', function () {
   });
 
   describe('#createOrUpdateIssue', () => {
-    before(async () => {
+    before(() => {
       github.MANAGED_LABELS = require('./labels.json');
     });
 
@@ -315,7 +322,7 @@ describe('GitHub', function () {
         });
 
         after(() => {
-          github.issuesCache.delete(EXISTING_CLOSED_ISSUE);
+          github.issuesCache.delete(EXISTING_CLOSED_ISSUE.title);
           github.issuesCache.set(EXISTING_CLOSED_ISSUE.title, EXISTING_CLOSED_ISSUE);
         });
 

src/reporter/index.js

@@ -50,7 +50,7 @@ export default class Reporter {
   }
 
   onTrackingStarted() {
-    return this.github.refreshIssuesCache();
+    return this.github.clearCache();
   }
 
   async onVersionRecorded(version) {