Skip to content

Commit

Permalink
fix: remember original state after LOADING (#10099)
Browse files Browse the repository at this point in the history 
Added method loadingFailed transitioning to CONNECTION_LOST if it
is the last active loading. Also removed the exposed
Vaadin.Flow.clients.TypeScript.loadingStarted / loadingFinished pair, as
blocking TB is anyway not working for multiple ongoing backend calls
(and it would be better handled by register ConnectionState as a Flow
client with isActive flag).
  • Loading branch information
@haijian-vaadin
Johannes Eriksson authored and haijian-vaadin committed Mar 26, 2021
1 parent d722352 commit b67cb42
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 101 deletions.
27 changes: 3 additions & 24 deletions flow-client/src/main/resources/META-INF/resources/frontend/Connect.ts
View file
Expand Up @@ -347,15 +347,14 @@ export class ConnectClient {
// chain item for our convenience. Always having an ending of the chain
// this way makes the folding down below more concise.
const fetchNext: MiddlewareNext = async (context: MiddlewareContext): Promise<Response> => {
this.loadingStarted();
$wnd.Vaadin.connectionState.loadingStarted();
return fetch(context.request)
.then((response) => {
this.loadingFinished();
$wnd.Vaadin.connectionState.loadingFinished();
return response;
})
.catch((error) => {
this.loadingFinished();
$wnd.Vaadin.connectionState.state = ConnectionState.CONNECTION_LOST;
$wnd.Vaadin.connectionState.loadingFailed();
return Promise.reject(error);
});
};
Expand Down Expand Up @@ -389,24 +388,4 @@ export class ConnectClient {
private isFlowLoaded(): boolean {
return $wnd.Vaadin.Flow?.clients?.TypeScript !== undefined;
}

private loadingStarted() {
if (this.isFlowLoaded()) {
// call Flow.loadingStarted to pause TestBench tests while backend
// requests are ongoing
$wnd.Vaadin.Flow.clients?.TypeScript?.loadingStarted();
} else {
$wnd.Vaadin.connectionState.loadingStarted();
}
}

private loadingFinished() {
if (this.isFlowLoaded()) {
// call Flow.loadingFinished to pause TestBench tests while backend
// requests are ongoing
$wnd.Vaadin.Flow.clients?.TypeScript?.loadingFinished();
} else {
$wnd.Vaadin.connectionState.loadingFinished();
}
}
}
10 changes: 9 additions & 1 deletion flow-client/src/main/resources/META-INF/resources/frontend/ConnectionState.ts
View file
Expand Up @@ -70,10 +70,18 @@ export class ConnectionStateStore {
}

loadingFinished(): void {
this.decreaseLoadingCount(ConnectionState.CONNECTED);
}

loadingFailed(): void {
this.decreaseLoadingCount(ConnectionState.CONNECTION_LOST);
}

private decreaseLoadingCount(finalState: ConnectionState) {
if (this.loadingCount > 0) {
this.loadingCount -= 1;
if (this.loadingCount === 0) {
this.state = ConnectionState.CONNECTED;
this.state = finalState;
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions flow-client/src/main/resources/META-INF/resources/frontend/Flow.ts
View file
Expand Up @@ -87,9 +87,7 @@ export class Flow {
$wnd.Vaadin.Flow = $wnd.Vaadin.Flow || {};
$wnd.Vaadin.Flow.clients = {
TypeScript: {
isActive: () => this.isActive,
loadingStarted: () => this.loadingStarted(),
loadingFinished: () => this.loadingFinished()
isActive: () => this.isActive
}
};

Expand Down
54 changes: 12 additions & 42 deletions flow-client/src/test/frontend/ConnectTests.ts
View file
Expand Up @@ -179,40 +179,19 @@ describe('ConnectClient', () => {
expect(fetchMock.lastOptions()).to.include({method: 'POST'});
});

it('should set CONNECTED followed by ConnectionState.loadingFinished when Flow is not loaded', async() => {
it('should set connection state to LOADING followed by CONNECTED on successful fetch', async() => {
let $wnd = (window as any);
let states: Array<ConnectionState> = [];
$wnd.Vaadin.connectionState.addStateChangeListener(
(_, state) => states.push(state));
await client.call('FooEndpoint', 'fooMethod');
expect(states).to.deep.equal([ConnectionState.LOADING, ConnectionState.CONNECTED]);
});
const stateChangeListener = sinon.fake();
$wnd.Vaadin.connectionState.addStateChangeListener(stateChangeListener);

it('should call Flow.loadingStarted followed by Flow.loadingFinished when Flow is loaded', async() => {
let calls: Array<boolean> = [];
(window as any).Vaadin.Flow = {
clients: {
TypeScript: {
loadingStarted: () => calls.push(true),
loadingFinished: () => calls.push(false)
}
}
};
await client.call('FooEndpoint', 'fooMethod');
expect(calls).to.deep.equal([true, false]);
(expect(stateChangeListener).to.be as any).calledWithExactly(ConnectionState.LOADING, ConnectionState.CONNECTED);
});

it('should call Flow.loadingFinished and transition to CONNECTION_LOST upon network failure', async() => {
let calls: Array<boolean> = [];
(window as any).Vaadin.Flow = {
clients: {
TypeScript: {
loadingStarted: () => calls.push(true),
loadingFinished: () => calls.push(false)
}
}
};

it('should set connection state to CONNECTION_LOST on network failure', async() => {
let $wnd = (window as any);
const stateChangeListener = sinon.fake();
$wnd.Vaadin.connectionState.addStateChangeListener(stateChangeListener);
fetchMock.post(
base + '/connect/FooEndpoint/reject',
Promise.reject(new TypeError('Network failure'))
Expand All @@ -222,12 +201,12 @@ describe('ConnectClient', () => {
} catch (error) {
// expected
} finally {
expect(calls).to.deep.equal([true, false]);
expect((window as any).Vaadin.connectionState.state).to.equal(ConnectionState.CONNECTION_LOST);
(expect(stateChangeListener).to.be as any).calledWithExactly(ConnectionState.LOADING, ConnectionState.CONNECTION_LOST);
}
});

it('should call Flow.loadingFinished upon server error', async() => {
it('should set connection state to CONNECTED upon server error', async() => {
let $wnd = (window as any);
const body = 'Unexpected error';
const errorResponse = new Response(
body,
Expand All @@ -238,21 +217,12 @@ describe('ConnectClient', () => {
);
fetchMock.post(base + '/connect/FooEndpoint/vaadinConnectResponse', errorResponse);

let calls: Array<boolean> = [];
(window as any).Vaadin.Flow = {
clients: {
TypeScript: {
loadingStarted: () => calls.push(true),
loadingFinished: () => calls.push(false)
}
}
};
try {
await client.call('FooEndpoint', 'vaadinConnectResponse');
} catch (error) {
// expected
} finally {
expect(calls).to.deep.equal([true, false]);
expect($wnd.Vaadin.connectionState.state).to.equal(ConnectionState.CONNECTED);
}
});

Expand Down
69 changes: 42 additions & 27 deletions flow-client/src/test/frontend/ConnectionStateTests.ts
View file
Expand Up @@ -12,38 +12,35 @@ describe('ConnectionStateStore', () => {

it('should call state change listeners when transitioning between states', () => {
const store = new ConnectionStateStore(ConnectionState.CONNECTED);
const stateChangeListener1 = sinon.spy((_: ConnectionState, __: ConnectionState) => {
});
const stateChangeListener2 = sinon.spy((_: ConnectionState, __: ConnectionState) => {
});

const stateChangeListener1 = sinon.fake();
const stateChangeListener2 = sinon.fake();
store.addStateChangeListener(stateChangeListener1);
store.addStateChangeListener(stateChangeListener2);

(expect(stateChangeListener1).to.not.be as any).called;
(expect(stateChangeListener2).to.not.be as any).called;

store.state = ConnectionState.CONNECTION_LOST;

(expect(stateChangeListener1).to.be as any).calledOnce;
(expect(stateChangeListener2).to.be as any).calledOnce;
});

it('should have removable state change listeners', () => {
const store = new ConnectionStateStore(ConnectionState.CONNECTED);
const stateChangeListener = sinon.spy((_: ConnectionState, __: ConnectionState) => {
});

const stateChangeListener = sinon.fake();
store.addStateChangeListener(stateChangeListener);

store.removeStateChangeListener(stateChangeListener);
store.state = ConnectionState.CONNECTION_LOST;
(expect(stateChangeListener).to.not.be as any).called;
});

it('state change listeners should be idempotent with respect to state update', () => {
const store = new ConnectionStateStore(ConnectionState.CONNECTED);
const stateChangeListener = sinon.spy((_: ConnectionState, __: ConnectionState) => {
});

const stateChangeListener = sinon.fake();
store.addStateChangeListener(stateChangeListener);

store.state = ConnectionState.CONNECTION_LOST;
store.state = ConnectionState.CONNECTION_LOST;

Expand All @@ -64,39 +61,57 @@ describe('ConnectionStateStore', () => {

it('LOADING states are stacked', () => {
const store = new ConnectionStateStore(ConnectionState.CONNECTED);
const states = Array<[ConnectionState,ConnectionState]>();
const stateChangeListener = (prev: ConnectionState, next: ConnectionState) => {
states.push([prev, next]);
};
const stateChangeListener = sinon.fake();
store.addStateChangeListener(stateChangeListener);

store.loadingStarted();
store.loadingStarted();
store.loadingFinished();
store.loadingFinished();
assert.deepEqual(states,
[[ConnectionState.CONNECTED, ConnectionState.LOADING],
[ConnectionState.LOADING, ConnectionState.CONNECTED]]);

assert.equal(stateChangeListener.callCount, 2);

(expect(stateChangeListener.getCall(0)).to.be as any).calledWithExactly(
ConnectionState.CONNECTED, ConnectionState.LOADING);
(expect(stateChangeListener.getCall(1)).to.be as any).calledWithExactly(
ConnectionState.LOADING, ConnectionState.CONNECTED);
});

it('loading count should reset when state forced', () => {
const store = new ConnectionStateStore(ConnectionState.CONNECTED);
const states = Array<[ConnectionState,ConnectionState]>();
const stateChangeListener = (prev: ConnectionState, next: ConnectionState) => {
states.push([prev, next]);
};
const stateChangeListener = sinon.fake();
store.addStateChangeListener(stateChangeListener);

store.loadingStarted();
store.state = ConnectionState.CONNECTION_LOST;
store.loadingStarted();
store.loadingFinished();

assert.deepEqual(states,
[[ConnectionState.CONNECTED, ConnectionState.LOADING],
[ConnectionState.LOADING, ConnectionState.CONNECTION_LOST],
[ConnectionState.CONNECTION_LOST, ConnectionState.LOADING],
[ConnectionState.LOADING, ConnectionState.CONNECTED]]);
assert.equal(stateChangeListener.callCount, 4);

(expect(stateChangeListener.getCall(0)).to.be as any).calledWithExactly(
ConnectionState.CONNECTED, ConnectionState.LOADING);
(expect(stateChangeListener.getCall(1)).to.be as any).calledWithExactly(
ConnectionState.LOADING, ConnectionState.CONNECTION_LOST);
(expect(stateChangeListener.getCall(2)).to.be as any).calledWithExactly(
ConnectionState.CONNECTION_LOST, ConnectionState.LOADING);
(expect(stateChangeListener.getCall(3)).to.be as any).calledWithExactly(
ConnectionState.LOADING, ConnectionState.CONNECTED);
});

it('loadingFailed should set state to CONNECTION_LOST', () => {
const store = new ConnectionStateStore(ConnectionState.CONNECTION_LOST);
const stateChangeListener = sinon.fake();
store.addStateChangeListener(stateChangeListener);

store.loadingStarted();
store.loadingFailed();

assert.equal(stateChangeListener.callCount, 2);
(expect(stateChangeListener.getCall(0)).to.be as any).calledWithExactly(
ConnectionState.CONNECTION_LOST, ConnectionState.LOADING);
(expect(stateChangeListener.getCall(1)).to.be as any).calledWithExactly(
ConnectionState.LOADING, ConnectionState.CONNECTION_LOST);
});

it('should request offline information from from service worker', async() => {
Expand Down
4 changes: 0 additions & 4 deletions flow-client/src/test/frontend/FlowTests.ts
View file
Expand Up @@ -323,10 +323,6 @@ suite("Flow", () => {
assert.isDefined($wnd.Vaadin.Flow.clients.TypeScript.isActive);
assert.isFalse($wnd.Vaadin.Flow.clients.TypeScript.isActive());

// Check that loadingStarted and loadingFinished are exposed
assert.isDefined($wnd.Vaadin.Flow.clients.TypeScript.loadingStarted);
assert.isDefined($wnd.Vaadin.Flow.clients.TypeScript.loadingFinished);

const route = flow.serverSideRoutes[0];

sinon.spy(flow, 'loadingStarted');
Expand Down

0 comments on commit b67cb42

Please sign in to comment.