From 5ac4ae37a26fa4cd7a558554c0ea1081791cbf7e Mon Sep 17 00:00:00 2001
From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com>
Date: Fri, 16 Oct 2020 16:45:38 +0200
Subject: [PATCH] BackendSrv: Fixes queue countdown when unsubscribe is before
 response (#28323) (#28328)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

(cherry picked from commit 9305117902a3698fcefc5d3063f58867717e34ce)

Co-authored-by: Hugo Häggmark <hugo.haggmark@grafana.com>
---
 public/app/core/services/ResponseQueue.test.ts | 11 ++++-------
 public/app/core/services/ResponseQueue.ts      | 14 ++------------
 public/app/core/services/backend_srv.ts        | 10 +++++++---
 3 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/public/app/core/services/ResponseQueue.test.ts b/public/app/core/services/ResponseQueue.test.ts
index 26f4fab9ef0..09f3aceffd1 100644
--- a/public/app/core/services/ResponseQueue.test.ts
+++ b/public/app/core/services/ResponseQueue.test.ts
@@ -26,30 +26,28 @@ const getTestContext = () => {
 
   const fetchMock = jest.fn().mockReturnValue(fetchResult);
   const setInProgressMock = jest.fn();
-  const setDoneMock = jest.fn();
 
   const queueMock: FetchQueue = ({
     add: jest.fn(),
     setInProgress: setInProgressMock,
-    setDone: setDoneMock,
+    setDone: jest.fn(),
     getUpdates: jest.fn(),
   } as unknown) as FetchQueue;
 
   const responseQueue = new ResponseQueue(queueMock, fetchMock);
 
-  return { id, options, expects, fetchMock, setInProgressMock, setDoneMock, responseQueue, fetchResult };
+  return { id, options, expects, fetchMock, setInProgressMock, responseQueue, fetchResult };
 };
 
 describe('ResponseQueue', () => {
   describe('add', () => {
     describe('when called', () => {
       it('then the matching fetchQueue entry should be set to inProgress', () => {
-        const { id, options, setInProgressMock, setDoneMock, responseQueue } = getTestContext();
+        const { id, options, setInProgressMock, responseQueue } = getTestContext();
 
         responseQueue.add(id, options);
 
         expect(setInProgressMock.mock.calls).toEqual([['id']]);
-        expect(setDoneMock).not.toHaveBeenCalled();
       });
 
       it('then a response entry with correct id should be published', done => {
@@ -81,14 +79,13 @@ describe('ResponseQueue', () => {
 
       describe('and when the fetch Observable is completed', () => {
         it('then the matching fetchQueue entry should be set to Done', done => {
-          const { id, options, responseQueue, setInProgressMock, setDoneMock } = getTestContext();
+          const { id, options, responseQueue, setInProgressMock } = getTestContext();
 
           subscribeTester({
             observable: responseQueue.getResponses(id).pipe(first()),
             expectCallback: data => {
               data.observable.subscribe().unsubscribe();
               expect(setInProgressMock.mock.calls).toEqual([['id']]);
-              expect(setDoneMock.mock.calls).toEqual([['id']]);
             },
             doneCallback: done,
           });
diff --git a/public/app/core/services/ResponseQueue.ts b/public/app/core/services/ResponseQueue.ts
index c3ab773db8e..8d281a04335 100644
--- a/public/app/core/services/ResponseQueue.ts
+++ b/public/app/core/services/ResponseQueue.ts
@@ -1,5 +1,5 @@
 import { Observable, Subject } from 'rxjs';
-import { filter, finalize } from 'rxjs/operators';
+import { filter } from 'rxjs/operators';
 import { BackendSrvRequest, FetchResponse } from '@grafana/runtime';
 import { FetchQueue } from './FetchQueue';
 
@@ -27,17 +27,7 @@ export class ResponseQueue {
       // Let the fetchQueue know that this id has started data fetching.
       fetchQueue.setInProgress(id);
 
-      this.responses.next({
-        id,
-        observable: fetch(options).pipe(
-          // finalize is called whenever this observable is unsubscribed/errored/completed/canceled
-          // https://rxjs.dev/api/operators/finalize
-          finalize(() => {
-            // Let the fetchQueue know that this id is done.
-            fetchQueue.setDone(id);
-          })
-        ),
-      });
+      this.responses.next({ id, observable: fetch(options) });
     });
   }
 
diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts
index 2ceeeb4cc02..ce2d9d1e17a 100644
--- a/public/app/core/services/backend_srv.ts
+++ b/public/app/core/services/backend_srv.ts
@@ -65,10 +65,11 @@ export class BackendSrv implements BackendService {
   }
 
   fetch<T>(options: BackendSrvRequest): Observable<FetchResponse<T>> {
-    return new Observable(observer => {
-      // We need to match an entry added to the queue stream with the entry that is eventually added to the response stream
-      const id = uuidv4();
+    // We need to match an entry added to the queue stream with the entry that is eventually added to the response stream
+    const id = uuidv4();
+    const fetchQueue = this.fetchQueue;
 
+    return new Observable(observer => {
       // Subscription is an object that is returned whenever you subscribe to an Observable.
       // You can also use it as a container of many subscriptions and when it is unsubscribed all subscriptions within are also unsubscribed.
       const subscriptions: Subscription = new Subscription();
@@ -89,6 +90,9 @@ export class BackendSrv implements BackendService {
 
       // This returned function will be called whenever the returned Observable from the fetch<T> function is unsubscribed/errored/completed/canceled.
       return function unsubscribe() {
+        // Change status to Done moved here from ResponseQueue because this unsubscribe was called before the responseQueue produced a result
+        fetchQueue.setDone(id);
+
         // When subscriptions is unsubscribed all the implicitly added subscriptions above are also unsubscribed.
         subscriptions.unsubscribe();
       };