From ae50bd62552994cbce7d1d907f37879aa4056732 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 19 Sep 2024 22:59:20 +0200 Subject: [PATCH 1/6] Add: GraphQL pagination to Decision Records --- DEVELOPMENT_DECISION_RECORDS.md | 4 ++++ doc/dev/decisionrecords/APIGraphQLDesign.md | 26 +++++++++++++++++++++ doc/dev/decisionrecords/_TEMPLATE.md | 2 -- 3 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 doc/dev/decisionrecords/APIGraphQLDesign.md diff --git a/DEVELOPMENT_DECISION_RECORDS.md b/DEVELOPMENT_DECISION_RECORDS.md index 10b9e005809..ef0085f7171 100644 --- a/DEVELOPMENT_DECISION_RECORDS.md +++ b/DEVELOPMENT_DECISION_RECORDS.md @@ -104,3 +104,7 @@ Prefer immutable types over mutable. Use builders where appropriate. See [Avoid using records if you cannot encapsulate it properly](doc/dev/decisionrecords/RecordsPOJOsBuilders.md#records) +## GraphQL Best Practices - API Design + +[Follow best practices for designing GraphQL APIs. Our APIs are used by hundreds of clients +(web-pages/apps/services) and need to backwards compatible.](doc/dev/A) \ No newline at end of file diff --git a/doc/dev/decisionrecords/APIGraphQLDesign.md b/doc/dev/decisionrecords/APIGraphQLDesign.md new file mode 100644 index 00000000000..f739497c14c --- /dev/null +++ b/doc/dev/decisionrecords/APIGraphQLDesign.md @@ -0,0 +1,26 @@ +# GraphQL Best Practices - API Design + +Follow best practices for designing GraphQL APIs. Our APIs are used by hundreds of clients +(web-pages/apps/services) and need to be backwards compatible. A good reference used by several +of the OTP developers are the Production Ready GraphQL book by Marc-André Giroux. + + +## Pagination + +We use the [pagination](https://graphql.org/learn/pagination/) (a.k. Relay) specification for paging over entities like station, +stops, trips and routes. Very often OTP has a _final_ list of entities in memory. For non-entities +(Itinerary and Legs), witch does not always have an ID and is none trivial to reconstruct, it is +better to make a custom solution. + + +## Consistency + +Unfortunately, part of the GraphQL API is old and does not follow best practices. So, when adding +new features, consider what is best; To follow the existing style or follow the best practice. + + +### Context and Problem Statement + +Our APIs are used by hundreds of clients(web-pages/apps/services) and need to backwards compatible. +Correcting mistakes may not be possible or may take a long time. + diff --git a/doc/dev/decisionrecords/_TEMPLATE.md b/doc/dev/decisionrecords/_TEMPLATE.md index 45bb3b11d34..e184bdc6aaa 100644 --- a/doc/dev/decisionrecords/_TEMPLATE.md +++ b/doc/dev/decisionrecords/_TEMPLATE.md @@ -6,8 +6,6 @@ later. --> -Original pull-request: {#NNNN} - ### Context and Problem Statement From f431d1734a189bc69e66ffed2cb3569d56aed822 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 24 Oct 2024 11:09:29 +0200 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: Leonard Ehrenfried --- DEVELOPMENT_DECISION_RECORDS.md | 2 +- doc/dev/decisionrecords/APIGraphQLDesign.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/DEVELOPMENT_DECISION_RECORDS.md b/DEVELOPMENT_DECISION_RECORDS.md index ef0085f7171..f5923092a94 100644 --- a/DEVELOPMENT_DECISION_RECORDS.md +++ b/DEVELOPMENT_DECISION_RECORDS.md @@ -107,4 +107,4 @@ Prefer immutable types over mutable. Use builders where appropriate. See ## GraphQL Best Practices - API Design [Follow best practices for designing GraphQL APIs. Our APIs are used by hundreds of clients -(web-pages/apps/services) and need to backwards compatible.](doc/dev/A) \ No newline at end of file +(web-pages/apps/services) and need to be backwards compatible.](doc/dev/A) \ No newline at end of file diff --git a/doc/dev/decisionrecords/APIGraphQLDesign.md b/doc/dev/decisionrecords/APIGraphQLDesign.md index f739497c14c..c9546ddf6f3 100644 --- a/doc/dev/decisionrecords/APIGraphQLDesign.md +++ b/doc/dev/decisionrecords/APIGraphQLDesign.md @@ -7,9 +7,9 @@ of the OTP developers are the Production Ready GraphQL book by Marc-André Girou ## Pagination -We use the [pagination](https://graphql.org/learn/pagination/) (a.k. Relay) specification for paging over entities like station, -stops, trips and routes. Very often OTP has a _final_ list of entities in memory. For non-entities -(Itinerary and Legs), witch does not always have an ID and is none trivial to reconstruct, it is +We use the [pagination](https://graphql.org/learn/pagination/) (a.k. Relay) specification for paging over entities like stations, +stops, trips and routes. Very often OTP has a _finite_ list of entities in memory. For non-entities +(Itinerary and Legs), witch do not always have an ID and is none trivial to reconstruct, it is better to make a custom solution. From 0f3f148b83e4464e8ae24186ca6660227e478677 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Mon, 9 Dec 2024 11:46:20 +0200 Subject: [PATCH 3/6] Fix link --- DEVELOPMENT_DECISION_RECORDS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DEVELOPMENT_DECISION_RECORDS.md b/DEVELOPMENT_DECISION_RECORDS.md index f5923092a94..b3766fad29d 100644 --- a/DEVELOPMENT_DECISION_RECORDS.md +++ b/DEVELOPMENT_DECISION_RECORDS.md @@ -107,4 +107,5 @@ Prefer immutable types over mutable. Use builders where appropriate. See ## GraphQL Best Practices - API Design [Follow best practices for designing GraphQL APIs. Our APIs are used by hundreds of clients -(web-pages/apps/services) and need to be backwards compatible.](doc/dev/A) \ No newline at end of file +(web-pages/apps/services) and need to be backwards compatible.]( +doc/dev/decisionrecords/APIGraphQLDesign.md) \ No newline at end of file From 3d9924cdfbc07e88a99acf273a38edda9a158caf Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Mon, 9 Dec 2024 11:49:36 +0200 Subject: [PATCH 4/6] Remove extra line break --- DEVELOPMENT_DECISION_RECORDS.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/DEVELOPMENT_DECISION_RECORDS.md b/DEVELOPMENT_DECISION_RECORDS.md index b3766fad29d..fc12950901b 100644 --- a/DEVELOPMENT_DECISION_RECORDS.md +++ b/DEVELOPMENT_DECISION_RECORDS.md @@ -107,5 +107,4 @@ Prefer immutable types over mutable. Use builders where appropriate. See ## GraphQL Best Practices - API Design [Follow best practices for designing GraphQL APIs. Our APIs are used by hundreds of clients -(web-pages/apps/services) and need to be backwards compatible.]( -doc/dev/decisionrecords/APIGraphQLDesign.md) \ No newline at end of file +(web-pages/apps/services) and need to be backwards compatible.](doc/dev/decisionrecords/APIGraphQLDesign.md) \ No newline at end of file From b22001acea630e1a016f8780186573bb07d692b1 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Mon, 9 Dec 2024 12:04:57 +0200 Subject: [PATCH 5/6] Add doc for refetching in GraphQL --- DEVELOPMENT_DECISION_RECORDS.md | 4 ++-- doc/dev/decisionrecords/APIGraphQLDesign.md | 21 ++++++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/DEVELOPMENT_DECISION_RECORDS.md b/DEVELOPMENT_DECISION_RECORDS.md index fc12950901b..a0e0554d4de 100644 --- a/DEVELOPMENT_DECISION_RECORDS.md +++ b/DEVELOPMENT_DECISION_RECORDS.md @@ -106,5 +106,5 @@ Prefer immutable types over mutable. Use builders where appropriate. See ## GraphQL Best Practices - API Design -[Follow best practices for designing GraphQL APIs. Our APIs are used by hundreds of clients -(web-pages/apps/services) and need to be backwards compatible.](doc/dev/decisionrecords/APIGraphQLDesign.md) \ No newline at end of file +[Follow best practices for designing GraphQL APIs. Our APIs need to be backwards compatible as they are used +by hundreds of clients (web-pages/apps/services).](doc/dev/decisionrecords/APIGraphQLDesign.md) \ No newline at end of file diff --git a/doc/dev/decisionrecords/APIGraphQLDesign.md b/doc/dev/decisionrecords/APIGraphQLDesign.md index c9546ddf6f3..1447f7b0c96 100644 --- a/doc/dev/decisionrecords/APIGraphQLDesign.md +++ b/doc/dev/decisionrecords/APIGraphQLDesign.md @@ -1,16 +1,23 @@ # GraphQL Best Practices - API Design -Follow best practices for designing GraphQL APIs. Our APIs are used by hundreds of clients -(web-pages/apps/services) and need to be backwards compatible. A good reference used by several -of the OTP developers are the Production Ready GraphQL book by Marc-André Giroux. +Follow best practices for designing GraphQL APIs. Our APIs need to be backwards compatible as they +are used by hundreds of clients (web-pages/apps/services). A good reference used by several +of the OTP developers is the Production Ready GraphQL book by Marc-André Giroux. ## Pagination We use the [pagination](https://graphql.org/learn/pagination/) (a.k. Relay) specification for paging over entities like stations, -stops, trips and routes. Very often OTP has a _finite_ list of entities in memory. For non-entities -(Itinerary and Legs), witch do not always have an ID and is none trivial to reconstruct, it is -better to make a custom solution. +stops, trips and routes. Very often OTP has a _finite_ list of entities in memory. + + +## Refetching + +We often use `type(id)` format queries for fetching or refetching an entities or value objects of type by id. Additionally, +the GTFS GraphQL API has a node interface and query for refetching objects which follow the +[GraphQL Global Object Identification Specification](https://relay.dev/graphql/objectidentification.htm). We should not use the +node interface or query for non-entities (such as Itineraries and Legs) which do not always have an ID and/or which IDs are not +trivial to reconstruct. ## Consistency @@ -21,6 +28,6 @@ new features, consider what is best; To follow the existing style or follow the ### Context and Problem Statement -Our APIs are used by hundreds of clients(web-pages/apps/services) and need to backwards compatible. +Our APIs need to be backwards compatible as they are used by hundreds of clients (web-pages/apps/services) Correcting mistakes may not be possible or may take a long time. From 2e7720ff090c52ffa91c3701476a63e0789dfa75 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Mon, 9 Dec 2024 12:41:20 +0200 Subject: [PATCH 6/6] Update doc/dev/decisionrecords/APIGraphQLDesign.md Co-authored-by: Leonard Ehrenfried --- doc/dev/decisionrecords/APIGraphQLDesign.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/dev/decisionrecords/APIGraphQLDesign.md b/doc/dev/decisionrecords/APIGraphQLDesign.md index 1447f7b0c96..c4dfc6fbd5e 100644 --- a/doc/dev/decisionrecords/APIGraphQLDesign.md +++ b/doc/dev/decisionrecords/APIGraphQLDesign.md @@ -13,7 +13,7 @@ stops, trips and routes. Very often OTP has a _finite_ list of entities in memor ## Refetching -We often use `type(id)` format queries for fetching or refetching an entities or value objects of type by id. Additionally, +We often use `type(id)` format queries for fetching or refetching entities or value objects of type by id. Additionally, the GTFS GraphQL API has a node interface and query for refetching objects which follow the [GraphQL Global Object Identification Specification](https://relay.dev/graphql/objectidentification.htm). We should not use the node interface or query for non-entities (such as Itineraries and Legs) which do not always have an ID and/or which IDs are not