From 5b86a850754d21e223cdd5885ff87798459c478c Mon Sep 17 00:00:00 2001 From: Hugo Ribeira Date: Mon, 4 Jul 2022 14:07:52 +0100 Subject: [PATCH] Fix race condition between removing a flight and updating it --- pages/index.tsx | 12 ++-- src/flight_analysis/components/analysis.tsx | 4 +- src/flight_analysis/components/index.tsx | 6 +- .../components/main_screen/map.tsx | 3 +- .../components/upload_screen.tsx | 14 ++-- src/flight_analysis/igc_blob.ts | 10 ++- src/flight_analysis/maps/flight_renderer.ts | 17 +++-- src/flight_analysis/maps/renderer.ts | 1 + src/flight_analysis/store/actions.ts | 15 ++-- src/flight_analysis/store/index.tsx | 11 +-- .../store/models/flight_datum.ts | 21 ++++++ src/flight_analysis/store/reducer/index.ts | 72 +++++++++++-------- .../url_flight_loaders/bga_loader.ts | 11 ++- .../url_flight_loaders/igc_loader.ts | 13 ++-- .../url_flight_loaders/loader.ts | 4 +- 15 files changed, 129 insertions(+), 85 deletions(-) create mode 100644 src/flight_analysis/store/models/flight_datum.ts diff --git a/pages/index.tsx b/pages/index.tsx index 59bad9f..932dc36 100644 --- a/pages/index.tsx +++ b/pages/index.tsx @@ -4,13 +4,13 @@ import LoadingScreen from "../src/flight_analysis/components/loading_screen"; import FullScreenWithDrawer from "../src/ui/components/layout/full_screen_with_drawer"; import BGALoader from "../src/flight_analysis/url_flight_loaders/bga_loader"; import IGCLoader from "../src/flight_analysis/url_flight_loaders/igc_loader"; -import FlightGroup from "glana/src/analysis/flight_group"; +import { FlightDatum } from "../src/flight_analysis/store/models/flight_datum"; const LOADER_CLASSES = [BGALoader, IGCLoader]; interface State { isLoading: boolean; - flightGroup?: FlightGroup; + flightData?: FlightDatum[]; } export default function Index() { @@ -24,7 +24,7 @@ export default function Index() { return state.isLoading ? ( ) : ( - + ); } @@ -36,14 +36,14 @@ async function maybeLoadFlightsFromURL(setState: any) { const loader = loaders.find(l => l.willHandle()); if (loader) { - const flightGroup = await loader.loadFlightGroup(); - if (flightGroup.flights.length < 1) { + const flightData = await loader.loadFlightGroup(); + if (flightData.length < 1) { window.location.href = "/failed_to_load_flight"; } setState((s: State) => ({ ...s, isLoading: false, - flightGroup: flightGroup + flightData: flightData })); } else { setState((s: State) => ({ ...s, isLoading: false })); diff --git a/src/flight_analysis/components/analysis.tsx b/src/flight_analysis/components/analysis.tsx index f6e8e51..210b5bc 100644 --- a/src/flight_analysis/components/analysis.tsx +++ b/src/flight_analysis/components/analysis.tsx @@ -29,8 +29,8 @@ export default function Analysis() { try { const igcBlob = new IGCBlob(files); - const flightGroup = await igcBlob.toFlightGroup(); - dispatch(actions.setFlightGroup(flightGroup)); + const flightData = await igcBlob.toFlightData(); + dispatch(actions.setFlightData(flightData)); } catch (e) { await errorTracker.report(e); } diff --git a/src/flight_analysis/components/index.tsx b/src/flight_analysis/components/index.tsx index 635db54..83e1996 100644 --- a/src/flight_analysis/components/index.tsx +++ b/src/flight_analysis/components/index.tsx @@ -1,15 +1,15 @@ import Analysis from "./analysis"; import React from "react"; import { StoreProvider } from "../store"; -import FlightGroup from "glana/src/analysis/flight_group"; +import { FlightDatum } from "../store/models/flight_datum"; interface Props { - flightGroup?: FlightGroup; + flightData?: FlightDatum[]; } export default function FlightAnalysis(props: Props) { return ( - + ); diff --git a/src/flight_analysis/components/main_screen/map.tsx b/src/flight_analysis/components/main_screen/map.tsx index ffee407..22486f1 100644 --- a/src/flight_analysis/components/main_screen/map.tsx +++ b/src/flight_analysis/components/main_screen/map.tsx @@ -101,7 +101,8 @@ function useRenderFlights( useEffect(() => { if (!mapRenderer || !flightData) return; flightData.forEach(f => mapRenderer.addFlight(f)); - mapRenderer.zoomToFit(); + // Give the UI time to hide the sidebar if it's open. + setTimeout(() => mapRenderer.zoomToFit(), 1000); return () => flightData.forEach(f => mapRenderer.removeFlight(f)); }, [mapRenderer, flightData]); diff --git a/src/flight_analysis/components/upload_screen.tsx b/src/flight_analysis/components/upload_screen.tsx index e0cdabe..fdfea70 100644 --- a/src/flight_analysis/components/upload_screen.tsx +++ b/src/flight_analysis/components/upload_screen.tsx @@ -13,10 +13,10 @@ export default function UploadScreen() { const onFileInput = async (event: React.ChangeEvent) => { event.preventDefault(); setIsLoading(true); - const flightGroup = await handleFileInput(event); - if (flightGroup) { + const flightData = await handleFileInput(event); + if (flightData.length > 0) { dispatch(actions.closeDrawer()); - dispatch(actions.setFlightGroup(flightGroup)); + dispatch(actions.setFlightData(flightData)); } else { setIsLoading(false); } @@ -79,16 +79,16 @@ export default function UploadScreen() { } async function handleFileInput(event: React.ChangeEvent) { - if (!event.target.files) return; + if (!event.target.files) return []; const files = Array.from(event.target.files) as Blob[]; - if (files.length < 1) return; + if (files.length < 1) return []; try { const igcBlob = new IGCBlob(files); - return await igcBlob.toFlightGroup(); + return await igcBlob.toFlightData(); } catch (e) { await errorTracker.report(e); - return null; + return []; } } diff --git a/src/flight_analysis/igc_blob.ts b/src/flight_analysis/igc_blob.ts index 45213c7..1cdb531 100644 --- a/src/flight_analysis/igc_blob.ts +++ b/src/flight_analysis/igc_blob.ts @@ -1,5 +1,5 @@ import IGCParser from "glana/src/igc/parser"; -import FlightGroup from "glana/src/analysis/flight_group"; +import { FlightDatum } from "./store/models/flight_datum"; export default class IGCBlob { private blobs: Blob[]; @@ -8,7 +8,7 @@ export default class IGCBlob { this.blobs = blobs; } - async toFlightGroup() { + async toFlightData() { let blobContents = await this.readBlobs(this.blobs); return this.parseIGCs(blobContents); } @@ -29,12 +29,10 @@ export default class IGCBlob { } private parseIGCs(blobContents: string[]) { - let savedFlights = blobContents.map(contents => { + return blobContents.map(contents => { let parser = new IGCParser(); const flight = parser.parse(contents); - return flight; + return new FlightDatum(flight); }); - - return new FlightGroup(savedFlights); } } diff --git a/src/flight_analysis/maps/flight_renderer.ts b/src/flight_analysis/maps/flight_renderer.ts index 8dd0a28..6dfaa14 100644 --- a/src/flight_analysis/maps/flight_renderer.ts +++ b/src/flight_analysis/maps/flight_renderer.ts @@ -38,6 +38,7 @@ export default class FlightRenderer { private timestamp: Date; private marker: mapboxgl.Marker; private trackSegments: TrackSegment[]; + private isDestroyed: boolean; flightDatum: FlightDatum; currentPosition: LngLatLike; @@ -46,8 +47,8 @@ export default class FlightRenderer { this.map = map; this.flightDatum = flightDatum; this.timestamp = flightDatum.flight.getRecordingStartedAt(); - this.sourceId = `source-${this.flightDatum.id}`; - this.layerId = `layer-${this.flightDatum.id}`; + this.sourceId = `source-flight-${this.flightDatum.id}`; + this.layerId = `layer-flight-${this.flightDatum.id}`; this.trackSegments = this.buildTrackSegments(flightDatum); this.geoJSON = this.buildGeoJSON(this.trackSegments); this.bounds = this.calculateBounds(this.geoJSON); @@ -168,6 +169,10 @@ export default class FlightRenderer { } destroy() { + if (this.isDestroyed) return; + + this.isDestroyed = true; + this.marker.remove(); this.map.removeLayer(this.layerId); this.map.removeSource(this.sourceId); @@ -178,6 +183,8 @@ export default class FlightRenderer { } setActive(isActive: boolean) { + if (this.isDestroyed) return; + this.isActive = isActive; this.setSourceOpacity(); this.setLayerZIndex(); @@ -188,7 +195,7 @@ export default class FlightRenderer { this.geoJSON.features.forEach(f => { f.properties.opacity = this.opacity; }); - source.setData(this.geoJSON); + source && source.setData(this.geoJSON); } private setLayerZIndex() { @@ -209,6 +216,8 @@ export default class FlightRenderer { } setTime(timestamp: Date) { + if (this.isDestroyed) return; + const datumIdx = this.flightDatum.flight.datumIndexAt(timestamp); const datum = this.flightDatum.flight.datums[datumIdx]; this.currentPosition = this.positionToGeoJSON(datum.position); @@ -244,7 +253,7 @@ export default class FlightRenderer { }); const source = this.map.getSource(this.sourceId) as GeoJSONSource; - source.setData(this.geoJSON); + source && source.setData(this.geoJSON); } private updateMarker(datum: Datum) { diff --git a/src/flight_analysis/maps/renderer.ts b/src/flight_analysis/maps/renderer.ts index 541638f..f77babe 100644 --- a/src/flight_analysis/maps/renderer.ts +++ b/src/flight_analysis/maps/renderer.ts @@ -296,6 +296,7 @@ export default class Renderer { const flightRenderer = this.flightRenderers[flightDatum.id]; if (!flightRenderer) return; flightRenderer.destroy(); + delete this.flightRenderers[flightDatum.id]; console.log(`Flight removed id=${flightDatum.id}`); } diff --git a/src/flight_analysis/store/actions.ts b/src/flight_analysis/store/actions.ts index 77ef09e..2afe6fa 100644 --- a/src/flight_analysis/store/actions.ts +++ b/src/flight_analysis/store/actions.ts @@ -1,12 +1,11 @@ -import FlightGroup from "glana/src/analysis/flight_group"; import { Duration, milliseconds } from "glana/src/units/duration"; import Quantity from "glana/src/units/quantity"; import analytics from "../../analytics"; import { Settings } from "../settings"; -import { FlightDatum } from "./reducer"; +import { FlightDatum } from "./models/flight_datum"; export enum ActionType { - SetFlightGroup = "SET_FLIGHT_GROUP", + SetFlightData = "SET_FLIGHT_DATA", SetActiveTimestamp = "SET_ACTIVE_TIMESTAMP", TogglePlay = "TOGGLE_PLAY", ToggleFlights = "TOGGLE_FLIGHTS", @@ -20,7 +19,7 @@ export enum ActionType { } export const actions = { - setFlightGroup, + setFlightData, setActiveTimestamp, advanceActiveTimestamp, togglePlay, @@ -43,13 +42,13 @@ type ElementType> = T extends ReadonlyArray< export type Action = ReturnType>; -function setFlightGroup(flightGroup: FlightGroup) { +function setFlightData(flightData: FlightDatum[]) { analytics.trackEvent("loaded_flights", { - count: flightGroup.flights.length + count: flightData.length }); return { - type: ActionType.SetFlightGroup as ActionType.SetFlightGroup, - flightGroup: flightGroup + type: ActionType.SetFlightData as ActionType.SetFlightData, + flightData: flightData }; } diff --git a/src/flight_analysis/store/index.tsx b/src/flight_analysis/store/index.tsx index 471767d..27ba78b 100644 --- a/src/flight_analysis/store/index.tsx +++ b/src/flight_analysis/store/index.tsx @@ -2,21 +2,22 @@ import React, { ReactNode, useContext, useEffect, useReducer } from "react"; import { initialState, reducer } from "./reducer"; import { Action, actions } from "./actions"; import FlightGroup from "glana/src/analysis/flight_group"; +import { FlightDatum } from "./models/flight_datum"; const StateContext = React.createContext(initialState()); const DispatchContent = React.createContext>(() => {}); interface ProviderProps { - flightGroup?: FlightGroup; + flightData?: FlightDatum[]; children: ReactNode; } export function StoreProvider(props: ProviderProps) { - const { flightGroup } = props; + const { flightData } = props; const [state, dispatch] = useReducer(reducer, initialState()); useEffect(() => { - if (flightGroup) { - dispatch(actions.setFlightGroup(flightGroup)); + if (flightData) { + dispatch(actions.setFlightData(flightData)); } else { dispatch(actions.showFlightUploader()); } @@ -27,7 +28,7 @@ export function StoreProvider(props: ProviderProps) { setDebug: (isDebug: boolean) => dispatch(actions.setDebug(isDebug)) }; } - }, [flightGroup]); + }, [flightData]); return ( diff --git a/src/flight_analysis/store/models/flight_datum.ts b/src/flight_analysis/store/models/flight_datum.ts new file mode 100644 index 0000000..af0df2a --- /dev/null +++ b/src/flight_analysis/store/models/flight_datum.ts @@ -0,0 +1,21 @@ +import Task from "glana/src/flight_computer/tasks/task"; +import SavedFlight from "glana/src/saved_flight"; +import { Picture } from "../reducer"; + +export class FlightDatum { + id: string; + flight: SavedFlight; + color: string; + pictures: Picture[]; + + constructor(savedFlight: SavedFlight) { + this.id = savedFlight.id; + this.flight = savedFlight; + this.color = "#000000"; + this.pictures = []; + } + + get task(): Task | null { + return this.flight.task; + } +} diff --git a/src/flight_analysis/store/reducer/index.ts b/src/flight_analysis/store/reducer/index.ts index 6437a2b..0ae601d 100644 --- a/src/flight_analysis/store/reducer/index.ts +++ b/src/flight_analysis/store/reducer/index.ts @@ -18,6 +18,13 @@ export type FlightDatum = { id: string; flight: SavedFlight; color: string; + pictures: Picture[]; +}; + +export type Picture = { + title: string; + url: string; + takenAt: Date; }; export type FlightDataById = { [key: string]: FlightDatum }; @@ -58,33 +65,8 @@ export function reducer(state: State, action: Action): State { let newSideDrawer: DrawerState | null; switch (action.type) { - case ActionType.SetFlightGroup: - const { flightGroup } = action; - const task = flightGroup.flights.map(f => f.task).find(t => !!t) || null; - - if (task) { - flightGroup.flights.forEach(f => (f.task = new Task(task.turnpoints))); - } - - let synchronizationMethod = state.settings.synchronizationMethod; - if ( - !flightGroup.allFlightsInSameDay() && - synchronizationMethod === synchronizationMethods.realTime - ) { - synchronizationMethod = synchronizationMethods.takeOff; - } - - newState = { - ...state, - settings: { ...state.settings, synchronizationMethod }, - isLoading: false, - sideDrawer: null - }; - - const analysis = buildAnalysisState(newState, flightGroup, task); - newState.analysis = analysis; - - return newState; + case ActionType.SetFlightData: + return handleSetFlightData(state, action); case ActionType.SetActiveTimestamp: if (!state.analysis) return state; @@ -225,6 +207,39 @@ export function reducer(state: State, action: Action): State { } } +function handleSetFlightData(state: State, action: Action) { + if (action.type !== ActionType.SetFlightData) return state; // hack to get types to work + + const { flightData } = action; + const task = flightData.map(f => f.task).find(t => !!t) || null; + + if (task) { + flightData.forEach(f => (f.flight.task = new Task(task!.turnpoints))); + } + + const flightGroup = new FlightGroup(flightData.map(f => f.flight)); + + let synchronizationMethod = state.settings.synchronizationMethod; + if ( + !flightGroup.allFlightsInSameDay() && + synchronizationMethod === synchronizationMethods.realTime + ) { + synchronizationMethod = synchronizationMethods.takeOff; + } + + const newState = { + ...state, + settings: { ...state.settings, synchronizationMethod }, + isLoading: false, + sideDrawer: null + }; + + const analysis = buildAnalysisState(newState, flightGroup, task); + newState.analysis = analysis; + + return newState; +} + function buildAnalysisState( state: State, flightGroup: FlightGroup, @@ -237,7 +252,8 @@ function buildAnalysisState( const flightData = flightGroup.flights.map(flight => ({ id: flight.id, flight: flight, - color: colors.nextColor() + color: colors.nextColor(), + pictures: [] })); const flightDataById = flightData.reduce((byId: FlightDataById, data) => { diff --git a/src/flight_analysis/url_flight_loaders/bga_loader.ts b/src/flight_analysis/url_flight_loaders/bga_loader.ts index abd5bcf..67f3296 100644 --- a/src/flight_analysis/url_flight_loaders/bga_loader.ts +++ b/src/flight_analysis/url_flight_loaders/bga_loader.ts @@ -1,4 +1,3 @@ -import FlightGroup from "glana/src/analysis/flight_group"; import Position from "glana/src/flight_computer/position"; import Task from "glana/src/flight_computer/tasks/task"; import Line from "glana/src/flight_computer/tasks/turnpoints/segments/line"; @@ -11,6 +10,7 @@ import SavedFlight from "glana/src/saved_flight"; import { degrees } from "glana/src/units/angle"; import { kilometers } from "glana/src/units/length"; import errorTracker from "../../error_tracker"; +import { FlightDatum } from "../store/models/flight_datum"; import Loader from "./loader"; const DEFAULT_BGA_BASE_URL = new URL("https://bgaladder.net"); @@ -29,7 +29,7 @@ export default class BGALoader implements Loader { async loadFlightGroup() { const flightDetailsResponses = await this.loadFlightDetails(); - const savedFlights = flightDetailsResponses + return flightDetailsResponses .map(response => { try { return this.parseFlightDetails(response); @@ -37,9 +37,7 @@ export default class BGALoader implements Loader { return null; } }) - .filter(sf => !!sf) as SavedFlight[]; - - return new FlightGroup(savedFlights); + .filter(sf => !!sf) as FlightDatum[]; } private parseBaseURL(query: URLSearchParams) { @@ -85,7 +83,8 @@ export default class BGALoader implements Loader { private parseFlightDetails(json: any) { let parser = new IGCParser(); const flight = parser.parse(json.igcContents); - return this.enrichFlightData(flight, json); + this.enrichFlightData(flight, json); + return new FlightDatum(flight); } private enrichFlightData(flight: SavedFlight, bgaData: any) { diff --git a/src/flight_analysis/url_flight_loaders/igc_loader.ts b/src/flight_analysis/url_flight_loaders/igc_loader.ts index b6c4390..cbdf857 100644 --- a/src/flight_analysis/url_flight_loaders/igc_loader.ts +++ b/src/flight_analysis/url_flight_loaders/igc_loader.ts @@ -1,8 +1,7 @@ import Loader from "./loader"; -import FlightGroup from "glana/src/analysis/flight_group"; import IGCParser from "glana/src/igc/parser"; -import SavedFlight from "glana/src/saved_flight"; import errorTracker from "../../error_tracker"; +import { FlightDatum } from "../store/models/flight_datum"; export default class IGCLoader implements Loader { private urls: string[]; @@ -18,8 +17,7 @@ export default class IGCLoader implements Loader { async loadFlightGroup() { const igcs = await this.loadIGCs(); const nonEmptyIgcs = igcs.filter(igc => !!igc) as string[]; - const flights = this.parseIGCs(nonEmptyIgcs); - return new FlightGroup(flights.filter(f => !!f) as SavedFlight[]); + return this.parseIGCs(nonEmptyIgcs); } private parseQueryString(query: URLSearchParams) { @@ -52,12 +50,13 @@ export default class IGCLoader implements Loader { private parseIGCs(fileContents: string[]) { return fileContents.map(contents => { try { - let parser = new IGCParser(); - return parser.parse(contents); + const parser = new IGCParser(); + const flight = parser.parse(contents); + return new FlightDatum(flight); } catch (e) { errorTracker.report(e); return null; } - }); + }) as FlightDatum[]; } } diff --git a/src/flight_analysis/url_flight_loaders/loader.ts b/src/flight_analysis/url_flight_loaders/loader.ts index 1bb1f9a..a4b273b 100644 --- a/src/flight_analysis/url_flight_loaders/loader.ts +++ b/src/flight_analysis/url_flight_loaders/loader.ts @@ -1,6 +1,6 @@ -import FlightGroup from "glana/src/analysis/flight_group"; +import { FlightDatum } from "../store/reducer"; export default interface Loader { willHandle(): boolean; - loadFlightGroup(): Promise; + loadFlightGroup(): Promise; }