Implement SignalR as middleware for project export enhancement #817

imnasnainaec · 2020-11-17T16:07:45Z

Follow-up to #819

TODO:

Unhardcode URL path for signal (@johnthagen)
Ensure Docker build works (@johnthagen)
- Need to proxy to /hub or, perhaps better, move /hub to where all other backend paths are

This change is

…e and, when ExportStatus.InProgress, receives SignalR messages

codecov-io · 2020-11-17T17:44:44Z

Codecov Report

Merging #817 (4278b21) into master (16f08d4) will decrease coverage by 0.14%.
The diff coverage is 37.30%.

@@            Coverage Diff             @@
##           master     #817      +/-   ##
==========================================
- Coverage   51.39%   51.24%   -0.15%     
==========================================
  Files         239      241       +2     
  Lines        6546     6621      +75     
  Branches      420      424       +4     
==========================================
+ Hits         3364     3393      +29     
- Misses       2871     2914      +43     
- Partials      311      314       +3

Flag	Coverage Δ
backend	`55.84% <58.18%> (-0.08%)`	⬇️
frontend	`46.76% <21.12%> (-0.23%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
Backend/Startup.cs	`0.00% <0.00%> (ø)`
src/components/App/AppLoggedIn.tsx	`0.00% <ø> (ø)`
src/components/App/DefaultState.tsx	`100.00% <ø> (ø)`
src/components/App/SignalRHub.tsx	`0.00% <0.00%> (ø)`
src/components/AppBar/AppBarComponent.tsx	`100.00% <ø> (ø)`
.../components/ProjectExport/ExportProjectActions.tsx	`0.00% <0.00%> (ø)`
.../components/ProjectExport/ExportProjectReducer.tsx	`44.44% <0.00%> (ø)`
src/components/ProjectExport/index.tsx	`66.66% <ø> (ø)`
...nents/ProjectSettings/ProjectSettingsComponent.tsx	`0.00% <ø> (ø)`
...teSettings/ProjectManagement/ProjectManagement.tsx	`100.00% <ø> (ø)`
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16f08d4...4278b21. Read the comment docs.

… (need to fix dispatch)

…ntend; have backend signal when download is ready

johnthagen · 2020-11-20T18:30:53Z

Backend/Startup.cs, line 94 at r4 (raw file):

                        .AllowAnyHeader()
                        .AllowAnyMethod()
                        .WithOrigins("http://localhost:3000")

Will this affect production? (e.g. non-localhost deployment)

johnthagen · 2020-11-21T14:09:04Z

Backend/Startup.cs, line 94 at r4 (raw file):

Previously, jmgrady (Jim Grady) wrote…

This is what is in ./src/types/runtimeConfig.tsx:

  public baseUrl(): string {
    // TODO: Remove support for previous configuration solution when
    //  Docker-based installation is used in production.
    if (window.runtimeConfig.hasOwnProperty("baseUrl")) {
      return window.runtimeConfig.baseUrl;
    }

    let baseUrl = "";
    if (window.runtimeConfig.hasOwnProperty("useConnectionBaseUrlForApi")) {
      baseUrl = `${window.location.protocol}//${window.location.host}`;
    } else {
      baseUrl = defaultConfig.baseUrl;
    }
    return baseUrl;
  }

Does this meet your needs?

Done

johnthagen · 2020-11-21T14:15:12Z

src/components/App/SignalRHub.tsx, line 49 at r17 (raw file):

        }
      };
      connection

@imnasnainaec The export works when I test it, but I do see the following error logged to the console:

index.js:1 Error: Cannot start a HubConnection that is not in the 'Disconnected' state.
    at HubConnection.<anonymous> (HubConnection.ts:163)
    at step (HubConnection.ts:2)
    at Object.next (HubConnection.ts:2)
    at HubConnection.ts:2
    at new Promise (<anonymous>)
    at push../node_modules/@microsoft/signalr/dist/esm/HubConnection.js.__awaiter (HubConnection.ts:2)
    at HubConnection.startWithStateTransitions (HubConnection.ts:161)
    at HubConnection.start (HubConnection.ts:157)
    at SignalRHub.tsx:49
    at commitHookEffectListMount (react-dom.development.js:19731)
    at commitPassiveHookEffects (react-dom.development.js:19769)
    at HTMLUnknownElement.callCallback (react-dom.development.js:188)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:237)
    at invokeGuardedCallback (react-dom.development.js:292)
    at flushPassiveEffectsImpl (react-dom.development.js:22853)
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority$1 (react-dom.development.js:11039)
    at flushPassiveEffects (react-dom.development.js:22820)
    at performSyncWorkOnRoot (react-dom.development.js:21737)
    at react-dom.development.js:11089
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority$1 (react-dom.development.js:11039)
    at flushSyncCallbackQueueImpl (react-dom.development.js:11084)
    at flushSyncCallbackQueue (react-dom.development.js:11072)
    at batchedUpdates$1 (react-dom.development.js:21862)
    at Object.notify (Subscription.js:19)
    at Subscription.notifyNestedSubs (Subscription.js:92)
    at Subscription.handleChangeWrapper (Subscription.js:97)
    at Object.dispatch (redux.js:222)
    at e (<anonymous>:1:40553)
    at Object.dispatch (index.js:11)
    at dispatch (<anonymous>:1:28545)
    at ExportProjectActions.tsx:31
    at HubConnection.method (SignalRHub.tsx:45)
    at HubConnection.ts:634
    at Array.forEach (<anonymous>)
    at HubConnection.invokeClientMethod (HubConnection.ts:634)
    at HubConnection.processIncomingData (HubConnection.ts:530)
    at WebSocketTransport.HubConnection.connection.onreceive (HubConnection.ts:102)
    at WebSocket.webSocket.onmessage (WebSocketTransport.ts:94)
console.<computed> @ index.js:1
overrideMethod @ react_devtools_backend.js:2430
(anonymous) @ SignalRHub.tsx:49
Promise.catch (async)
(anonymous) @ SignalRHub.tsx:49
commitHookEffectListMount @ react-dom.development.js:19731
commitPassiveHookEffects @ react-dom.development.js:19769
callCallback @ react-dom.development.js:188
invokeGuardedCallbackDev @ react-dom.development.js:237
invokeGuardedCallback @ react-dom.development.js:292
flushPassiveEffectsImpl @ react-dom.development.js:22853
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushPassiveEffects @ react-dom.development.js:22820
performSyncWorkOnRoot @ react-dom.development.js:21737
(anonymous) @ react-dom.development.js:11089
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushSyncCallbackQueueImpl @ react-dom.development.js:11084
flushSyncCallbackQueue @ react-dom.development.js:11072
batchedUpdates$1 @ react-dom.development.js:21862
notify @ Subscription.js:19
notifyNestedSubs @ Subscription.js:92
handleChangeWrapper @ Subscription.js:97
dispatch @ redux.js:222
e @ VM347:1
(anonymous) @ index.js:11
dispatch @ VM347:1
(anonymous) @ ExportProjectActions.tsx:31
method @ SignalRHub.tsx:45
(anonymous) @ HubConnection.ts:634
HubConnection.invokeClientMethod @ HubConnection.ts:634
HubConnection.processIncomingData @ HubConnection.ts:530
HubConnection.connection.onreceive @ HubConnection.ts:102
webSocket.onmessage @ WebSocketTransport.ts:94

…le user (in case of front-end interruption of the first).

imnasnainaec

Reviewable status: 6 of 29 files reviewed, 1 unresolved discussion (waiting on @jasonleenaylor and @johnthagen)

src/components/App/SignalRHub.tsx, line 49 at r17 (raw file):

Previously, johnthagen wrote…

@imnasnainaec The export works when I test it, but I do see the following error logged to the console:

index.js:1 Error: Cannot start a HubConnection that is not in the 'Disconnected' state.
    at HubConnection.<anonymous> (HubConnection.ts:163)
    at step (HubConnection.ts:2)
    at Object.next (HubConnection.ts:2)
    at HubConnection.ts:2
    at new Promise (<anonymous>)
    at push../node_modules/@microsoft/signalr/dist/esm/HubConnection.js.__awaiter (HubConnection.ts:2)
    at HubConnection.startWithStateTransitions (HubConnection.ts:161)
    at HubConnection.start (HubConnection.ts:157)
    at SignalRHub.tsx:49
    at commitHookEffectListMount (react-dom.development.js:19731)
    at commitPassiveHookEffects (react-dom.development.js:19769)
    at HTMLUnknownElement.callCallback (react-dom.development.js:188)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:237)
    at invokeGuardedCallback (react-dom.development.js:292)
    at flushPassiveEffectsImpl (react-dom.development.js:22853)
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority$1 (react-dom.development.js:11039)
    at flushPassiveEffects (react-dom.development.js:22820)
    at performSyncWorkOnRoot (react-dom.development.js:21737)
    at react-dom.development.js:11089
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority$1 (react-dom.development.js:11039)
    at flushSyncCallbackQueueImpl (react-dom.development.js:11084)
    at flushSyncCallbackQueue (react-dom.development.js:11072)
    at batchedUpdates$1 (react-dom.development.js:21862)
    at Object.notify (Subscription.js:19)
    at Subscription.notifyNestedSubs (Subscription.js:92)
    at Subscription.handleChangeWrapper (Subscription.js:97)
    at Object.dispatch (redux.js:222)
    at e (<anonymous>:1:40553)
    at Object.dispatch (index.js:11)
    at dispatch (<anonymous>:1:28545)
    at ExportProjectActions.tsx:31
    at HubConnection.method (SignalRHub.tsx:45)
    at HubConnection.ts:634
    at Array.forEach (<anonymous>)
    at HubConnection.invokeClientMethod (HubConnection.ts:634)
    at HubConnection.processIncomingData (HubConnection.ts:530)
    at WebSocketTransport.HubConnection.connection.onreceive (HubConnection.ts:102)
    at WebSocket.webSocket.onmessage (WebSocketTransport.ts:94)
console.<computed> @ index.js:1
overrideMethod @ react_devtools_backend.js:2430
(anonymous) @ SignalRHub.tsx:49
Promise.catch (async)
(anonymous) @ SignalRHub.tsx:49
commitHookEffectListMount @ react-dom.development.js:19731
commitPassiveHookEffects @ react-dom.development.js:19769
callCallback @ react-dom.development.js:188
invokeGuardedCallbackDev @ react-dom.development.js:237
invokeGuardedCallback @ react-dom.development.js:292
flushPassiveEffectsImpl @ react-dom.development.js:22853
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushPassiveEffects @ react-dom.development.js:22820
performSyncWorkOnRoot @ react-dom.development.js:21737
(anonymous) @ react-dom.development.js:11089
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushSyncCallbackQueueImpl @ react-dom.development.js:11084
flushSyncCallbackQueue @ react-dom.development.js:11072
batchedUpdates$1 @ react-dom.development.js:21862
notify @ Subscription.js:19
notifyNestedSubs @ Subscription.js:92
handleChangeWrapper @ Subscription.js:97
dispatch @ redux.js:222
e @ VM347:1
(anonymous) @ index.js:11
dispatch @ VM347:1
(anonymous) @ ExportProjectActions.tsx:31
method @ SignalRHub.tsx:45
(anonymous) @ HubConnection.ts:634
HubConnection.invokeClientMethod @ HubConnection.ts:634
HubConnection.processIncomingData @ HubConnection.ts:530
HubConnection.connection.onreceive @ HubConnection.ts:102
webSocket.onmessage @ WebSocketTransport.ts:94

I believe that's resolved now.

lgtm-com · 2020-11-23T17:56:42Z

This pull request introduces 1 alert when merging 6f4db7f into 235c91c - view on LGTM.com

new alerts:

1 for Rethrowing exception variable

jasonleenaylor · 2020-11-23T18:28:27Z

Backend/Controllers/LiftController.cs, line 241 at r19 (raw file):

                return new OkObjectResult(projectId);
            }
            catch

Better to catch(Exception e) and throw e;

imnasnainaec

Reviewable status: 6 of 29 files reviewed, 1 unresolved discussion (waiting on @jasonleenaylor and @johnthagen)

Backend/Controllers/LiftController.cs, line 241 at r19 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

Better to catch(Exception e) and throw e;

That's what I'd done at first, but LGTM said it's bad practice, breaking call-stacks, and suggested this format. I assumed this way implicitly threw the error it caught.

jasonleenaylor · 2020-11-23T22:23:32Z

Backend/Controllers/LiftController.cs, line 241 at r19 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

That's what I'd done at first, but LGTM said it's bad practice, breaking call-stacks, and suggested this format. I assumed this way implicitly threw the error it caught.

So it does...lgtm taught me something new today.

jasonleenaylor · 2020-11-23T22:24:05Z

Backend/Services/LiftApiServices.cs, line 92 at r19 (raw file):

 private readonly string inProgress = "IN_PROGRESS";

For style purposes make this a const and move it to the top of the file. Unless LGTM doesn't like that either. ;)

imnasnainaec

Reviewable status: 6 of 29 files reviewed, 1 unresolved discussion (waiting on @jasonleenaylor and @johnthagen)

Backend/Services/LiftApiServices.cs, line 92 at r19 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

 private readonly string inProgress = "IN_PROGRESS";
For style purposes make this a const and move it to the top of the file. Unless LGTM doesn't like that either. ;)

Moved to top of class.

jasonleenaylor

Reviewed 9 of 24 files at r2, 1 of 4 files at r4, 1 of 6 files at r14, 1 of 2 files at r15, 1 of 3 files at r16, 1 of 3 files at r17, 7 of 9 files at r18, 2 of 2 files at r19, 1 of 1 files at r20.
Dismissed @johnthagen from a discussion.
Reviewable status: complete! all files reviewed, all discussions resolved

src/components/ProjectExport/ExportProjectButton.tsx, line 16 at r20 (raw file):

 utto

Colony collapse disorder must be affecting your code comments.

imnasnainaec

Reviewed 11 of 24 files at r2, 1 of 4 files at r4, 2 of 2 files at r6, 2 of 4 files at r7, 1 of 6 files at r14, 1 of 2 files at r15, 1 of 3 files at r16, 1 of 3 files at r17, 5 of 9 files at r18, 2 of 2 files at r19, 1 of 1 files at r20, 1 of 1 files at r21.
Reviewable status: complete! all files reviewed, all discussions resolved

imnasnainaec

Reviewed 2 of 2 files at r22.
Reviewable status: complete! all files reviewed, all discussions resolved

imnasnainaec added 2 commits November 11, 2020 11:35

Rudimentary SignalR implementation

f4401fb

Custom middleware

6babbdd

imnasnainaec added enhancement New feature or request backend frontend refactor import/export labels Nov 17, 2020

imnasnainaec self-assigned this Nov 17, 2020

imnasnainaec added 4 commits November 17, 2020 11:15

Merge in 'maser'

7326b3b

fmt

5e6baf6

Intercept

f94ade5

Add to AppBar a DownloadButton that is dynamically tied to exportStat…

2102ccd

…e and, when ExportStatus.InProgress, receives SignalR messages

imnasnainaec mentioned this pull request Nov 18, 2020

Split lift export function in backend #819

Merged

imnasnainaec added 4 commits November 18, 2020 16:22

Merge branch 'master' into signalr

f250e2f

Make ProjectExport move out; move download action into DownloadButton…

583c9e3

… (need to fix dispatch)

finish export/download split; extract signalR as own component in fro…

686f922

…ntend; have backend signal when download is ready

Merge in 'master'

064cd99

This comment has been minimized.

Sign in to view

imnasnainaec added 2 commits November 20, 2020 11:58

Split download and delete/reset

6444ee3

Merge in 'master'

24d3b46

sillsdev deleted a comment from lgtm-com bot Nov 20, 2020

This comment has been minimized.

Sign in to view

imnasnainaec added 3 commits November 20, 2020 13:16

Fix useEffect for turning on SignalR connection

e55fdf4

Add export reset to reducer

d4ee912

Improve connection logging

43e6b1b

This comment has been minimized.

Sign in to view

Remove dead code

39cf3e0

imnasnainaec and others added 3 commits November 20, 2020 16:20

Don't reuse global var name locally

be8bc2d

Unify the frontend usages of baseURL

14e318e

Support web socket in NGINX configuration

25a386e

imnasnainaec added 3 commits November 21, 2020 12:14

Comment cleanup

d7ea03c

Stop all connections if exportState changes.

203642a

Add ability to cancel in-progress exports.

e3a5e5d

imnasnainaec requested a review from jasonleenaylor November 21, 2020 18:22

imnasnainaec assigned johnthagen Nov 21, 2020

imnasnainaec added 2 commits November 23, 2020 12:08

Remove frontend cancel export, lacking backend to match.

bc84f77

Add backend inProgress status to prevent multiple exports from a sing…

6f4db7f

…le user (in case of front-end interruption of the first).

imnasnainaec commented Nov 23, 2020

View reviewed changes

imnasnainaec added 2 commits November 23, 2020 12:58

Removed unused translation.

057674d

Fix catch-throw.

b3903e5

imnasnainaec commented Nov 23, 2020

View reviewed changes

imnasnainaec marked this pull request as ready for review November 23, 2020 21:53

Change readonly to const at top of class.

c69e151

imnasnainaec commented Nov 23, 2020

View reviewed changes

jasonleenaylor approved these changes Nov 23, 2020

View reviewed changes

Cure colony collapse disorder.

61a6511

imnasnainaec commented Nov 23, 2020

View reviewed changes

Merge branch 'master' into signalr

4278b21

imnasnainaec commented Nov 24, 2020

View reviewed changes

imnasnainaec merged commit 0515bde into master Nov 24, 2020

imnasnainaec deleted the signalr branch November 24, 2020 16:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement SignalR as middleware for project export enhancement #817

Implement SignalR as middleware for project export enhancement #817

imnasnainaec commented Nov 17, 2020 •

edited by johnthagen

Loading

codecov-io commented Nov 17, 2020 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

johnthagen commented Nov 20, 2020

johnthagen commented Nov 21, 2020

johnthagen commented Nov 21, 2020

imnasnainaec left a comment

lgtm-com bot commented Nov 23, 2020

jasonleenaylor commented Nov 23, 2020

imnasnainaec left a comment

jasonleenaylor commented Nov 23, 2020

jasonleenaylor commented Nov 23, 2020

imnasnainaec left a comment

jasonleenaylor left a comment

imnasnainaec left a comment

imnasnainaec left a comment

Implement SignalR as middleware for project export enhancement #817

Implement SignalR as middleware for project export enhancement #817

Conversation

imnasnainaec commented Nov 17, 2020 • edited by johnthagen Loading

codecov-io commented Nov 17, 2020 • edited Loading

Codecov Report

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

johnthagen commented Nov 20, 2020

johnthagen commented Nov 21, 2020

johnthagen commented Nov 21, 2020

imnasnainaec left a comment

Choose a reason for hiding this comment

lgtm-com bot commented Nov 23, 2020

jasonleenaylor commented Nov 23, 2020

imnasnainaec left a comment

Choose a reason for hiding this comment

jasonleenaylor commented Nov 23, 2020

jasonleenaylor commented Nov 23, 2020

imnasnainaec left a comment

Choose a reason for hiding this comment

jasonleenaylor left a comment

Choose a reason for hiding this comment

imnasnainaec left a comment

Choose a reason for hiding this comment

imnasnainaec left a comment

Choose a reason for hiding this comment

imnasnainaec commented Nov 17, 2020 •

edited by johnthagen

Loading

codecov-io commented Nov 17, 2020 •

edited

Loading