Last active
December 22, 2017 00:05
-
-
Save GeorgDangl/730faceaa465fcf50f84f0d9471a2066 to your computer and use it in GitHub Desktop.
Chasing a bug in ng-lightquery - cache invalidation is hard! https://blog.dangl.me/archive/chasing-a-bug-when-working-with-an-rxjs-observable/
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
export class MainComponent { | |
onEditorClosed(hasChanged: boolean) { | |
if (hasChanged) { | |
this.dataService.forceRefresh(); | |
this.dataService.paginationResult | |
.skip(1) // Discard the current value from the ReplaySubject | |
.take(1) // To release the subscription again | |
.subscribe(() => this.dataService.getDataById(this.data.id) | |
.then(data => { | |
this.data = data; | |
})); | |
} | |
} | |
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
export class ExtendedService extends PaginationBaseService<Data> { | |
getDataById(id: string): Promise<Data> { | |
if (this.lastPaginationResult && this.lastPaginationResult.data) { | |
const cachedData = this.lastPaginationResult.data.find(v => v.id === id); | |
if (cachedData) { | |
return Promise.resolve(cachedData); | |
} | |
} | |
const url = `${this.baseUrl}/${id}`; | |
return this.http | |
.get(url) | |
.toPromise() | |
.then(r => r as Data); | |
} | |
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
let asyncStyleCacheCheck = (requeryAction: (UserService) => void) => { | |
// This is a regression for the following bug: | |
// A derived service did implement a getItemById() function which did, before making a | |
// network call to retrieve the resource from the server, check if the item with the given | |
// id is present in the 'lastPaginationResult' local cache. | |
// Now after a force refresh and a subsequent retrieval, the following sequence of events happened: | |
// 1. Component subscribes to the pagination result to be notified when an update was finished. | |
// 2. The same component did react to the subscribe event by getting an item by its id | |
// 3. The service refreshed the items and raised the events | |
// 4. The component reacted by getting the item by id, which retrived it from the (stale) local cache of the service | |
// 5. The service finished it's refresh action and updated the local cache | |
// This was happening because after emitting 'next()' on an observable, subscribed actions were executed before | |
// continuing with the current code path | |
// Something learned today! | |
let service = getService(); | |
service.baseUrl = '/users'; | |
let httpMock = getHttpMock(); | |
const initialUsers = [{ userName: 'George', id: 1 }, { userName: 'Dave', id: 2 }]; | |
const updatedUsers = [{ userName: 'Giorgio', id: 1 }, { userName: 'Dave', id: 2 }] | |
const initialRequest = httpMock.expectOne('/users?page=1&pageSize=20'); | |
const initialPaginationResponse: PaginationResult<User> = { | |
data: initialUsers, | |
page: 1, | |
pageSize: 20, | |
totalCount: 1 | |
}; | |
initialRequest.flush(initialPaginationResponse); | |
service.paginationResult | |
.skip(1) // To not get the initial value from the first request | |
.subscribe(usersPaginated => { | |
var hasNewUser = usersPaginated.data.find(u => u.id === 1 && u.userName === 'Giorgio'); | |
expect(hasNewUser).toBeTruthy(); | |
var newUserById = service.getUserById(1) | |
.then(user => { | |
expect(user.userName).toBe('Giorgio'); // Initially, this returned the user 'George' from the stale cache | |
}); | |
}); | |
requeryAction(service); // This is currently either calling 'forceRefresh()' or 'requery()' | |
const secondRequest = httpMock.expectOne('/users?page=1&pageSize=20'); | |
const secondPaginationResponse: PaginationResult<User> = { | |
data: updatedUsers, | |
page: 1, | |
pageSize: 20, | |
totalCount: 1 | |
}; | |
secondRequest.flush(secondPaginationResponse); | |
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
export abstract class PaginationBaseService<T> { | |
constructor(protected http: HttpClient) { | |
this.requestUrl.distinctUntilChanged() | |
.merge(this.forceRefreshUrl) | |
.switchMap((url: string) => { | |
return this.http.get<PaginationResult<T>>(url) | |
.catch(er => Observable.empty<PaginationResult<T>>()); | |
}) | |
.filter(r => r != null) | |
.subscribe(result => { | |
this.lastPaginationResult = result; | |
this.paginationResultSource.next(result); | |
}, error => { /* Does nothing on error */ }); | |
} | |
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
export abstract class PaginationBaseService<T> { | |
constructor(protected http: HttpClient) { | |
this.requestUrl.distinctUntilChanged() | |
.merge(this.forceRefreshUrl) | |
.switchMap((url: string) => { | |
return this.http.get<PaginationResult<T>>(url) | |
.catch(er => Observable.empty<PaginationResult<T>>()); | |
}) | |
.filter(r => r != null) | |
.subscribe(result => { | |
this.paginationResultSource.next(result); | |
this.lastPaginationResult = result; | |
}, error => { /* Does nothing on error */ }); | |
} | |
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment