From b7e0460d26953f5a41240f0466c10cc2c1888b39 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Thu, 26 Sep 2024 19:19:49 +0300 Subject: [PATCH 1/9] Clarify @Nullable and @Nonnull usage --- doc/dev/decisionrecords/Codestyle.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/dev/decisionrecords/Codestyle.md b/doc/dev/decisionrecords/Codestyle.md index f9ffc1a9056..b302ce2b4a6 100644 --- a/doc/dev/decisionrecords/Codestyle.md +++ b/doc/dev/decisionrecords/Codestyle.md @@ -158,6 +158,13 @@ What to put in Javadoc: - Is it immutable or should anything be treated as immutable - Is it a utility class of static methods that should not be instantiated +### Annotations + +- On methods: + - Method should be marked as `@Nullable` if it can return null values + - Use of `@Nonnull` on methods should be avoided as it's the default behaviour + - Method parameters can be marked as `@Nullable` or `@Nonnull` + ## JavaScript As of #206, we From 3ad5c228d7056866f2ccc78da9b4241c429e74fd Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Tue, 8 Oct 2024 15:17:43 +0300 Subject: [PATCH 2/9] Update transit model readme --- src/main/java/org/opentripplanner/transit/model/package.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opentripplanner/transit/model/package.md b/src/main/java/org/opentripplanner/transit/model/package.md index 9c10c88828c..98df088bec4 100644 --- a/src/main/java/org/opentripplanner/transit/model/package.md +++ b/src/main/java/org/opentripplanner/transit/model/package.md @@ -23,10 +23,10 @@ All transit entities must have an ID. Transit entities ar "root" level are consi roots_. -#### @Nonnull and @Nullable entity fields +#### Non-null and nullable entity fields -All fields getters(except primitive types) should be annotated with `@Nullable` or `@Nonnull`. None -null field should be enforced in the Entity constructor by using `Objects.requireNonNull`, +All fields getters(except primitive types) should be annotated with `@Nullable` if they can return null. +Non-nullability of fields should be enforced in the Entity constructor by using `Objects.requireNonNull`, `Objects.requireNonNullElse` or `ObjectUtils.ifNotNull`. We should enforce this for all fields required in both GTFS and in the Nordic NeTEx Profile. For enumeration types using a special value like `UNKNOWN` is preferred over making the field optional. From d1bbfff98c6d20b361bdab9221929f864136639e Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Tue, 8 Oct 2024 16:52:59 +0300 Subject: [PATCH 3/9] Further clarify @Nonnull and @Nullable usage --- doc/dev/decisionrecords/Codestyle.md | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/doc/dev/decisionrecords/Codestyle.md b/doc/dev/decisionrecords/Codestyle.md index b302ce2b4a6..7c2dcf7688c 100644 --- a/doc/dev/decisionrecords/Codestyle.md +++ b/doc/dev/decisionrecords/Codestyle.md @@ -161,9 +161,17 @@ What to put in Javadoc: ### Annotations - On methods: - - Method should be marked as `@Nullable` if it can return null values - - Use of `@Nonnull` on methods should be avoided as it's the default behaviour - - Method parameters can be marked as `@Nullable` or `@Nonnull` + - Method should be marked as `@Nullable` if they can return null values + - Method parameters should be marked as `@Nullable`. +- On fields: + - Fields should often be marked as `@Nullable` if they are nullable. If the class + only exposes the field through a simple getter method, using the annotation on field is optional, + but on the getter method it is required. + +Use of `@Nonnull` annotation is not allowed. It should be assumed methods/parameters/fields +are non-null if they are not marked as `@Nullable`. However, there are places where the +`@Nullable` annotation is missing even if it should have been used. Those can be updated +to use the `@Nullable` annotation. ## JavaScript From 83d8e3e3c0ffc52eb85710403aacf15774d51ca6 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Wed, 9 Oct 2024 11:05:11 +0300 Subject: [PATCH 4/9] Update doc/dev/decisionrecords/Codestyle.md Co-authored-by: Henrik Abrahamsson <127481124+habrahamsson-skanetrafiken@users.noreply.github.com> --- doc/dev/decisionrecords/Codestyle.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/dev/decisionrecords/Codestyle.md b/doc/dev/decisionrecords/Codestyle.md index 7c2dcf7688c..be777e36337 100644 --- a/doc/dev/decisionrecords/Codestyle.md +++ b/doc/dev/decisionrecords/Codestyle.md @@ -162,7 +162,7 @@ What to put in Javadoc: - On methods: - Method should be marked as `@Nullable` if they can return null values - - Method parameters should be marked as `@Nullable`. + - Method parameters should be marked as `@Nullable` if they can take null values. - On fields: - Fields should often be marked as `@Nullable` if they are nullable. If the class only exposes the field through a simple getter method, using the annotation on field is optional, From 9b1a57707d2522a79f6998af91ef260ed8da3205 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Thu, 10 Oct 2024 16:43:55 +0300 Subject: [PATCH 5/9] Require use of @Nullable on fields and remove annotations from transit model readme --- doc/dev/decisionrecords/Codestyle.md | 4 +--- src/main/java/org/opentripplanner/transit/model/package.md | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/doc/dev/decisionrecords/Codestyle.md b/doc/dev/decisionrecords/Codestyle.md index be777e36337..c554e3c9cd9 100644 --- a/doc/dev/decisionrecords/Codestyle.md +++ b/doc/dev/decisionrecords/Codestyle.md @@ -164,9 +164,7 @@ What to put in Javadoc: - Method should be marked as `@Nullable` if they can return null values - Method parameters should be marked as `@Nullable` if they can take null values. - On fields: - - Fields should often be marked as `@Nullable` if they are nullable. If the class - only exposes the field through a simple getter method, using the annotation on field is optional, - but on the getter method it is required. + - Fields should be marked as `@Nullable` if they are nullable. Use of `@Nonnull` annotation is not allowed. It should be assumed methods/parameters/fields are non-null if they are not marked as `@Nullable`. However, there are places where the diff --git a/src/main/java/org/opentripplanner/transit/model/package.md b/src/main/java/org/opentripplanner/transit/model/package.md index 98df088bec4..26ed7facde3 100644 --- a/src/main/java/org/opentripplanner/transit/model/package.md +++ b/src/main/java/org/opentripplanner/transit/model/package.md @@ -25,7 +25,6 @@ roots_. #### Non-null and nullable entity fields -All fields getters(except primitive types) should be annotated with `@Nullable` if they can return null. Non-nullability of fields should be enforced in the Entity constructor by using `Objects.requireNonNull`, `Objects.requireNonNullElse` or `ObjectUtils.ifNotNull`. We should enforce this for all fields required in both GTFS and in the Nordic NeTEx Profile. For enumeration types using a special value From c8d07e0550ea626402d98bf9f6157a095aaa51fb Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Fri, 11 Oct 2024 13:58:57 +0300 Subject: [PATCH 6/9] Transform package-info java files to md and remove some --- .../ext/datastore/gs/package-info.java | 8 ------- .../ext/datastore/gs/package-info.md | 6 +++++ .../ext/restapi/resources/package-info.java | 5 ----- .../ext/restapi/resources/package-info.md | 4 ++++ .../api/parameter/package-info.java | 6 ----- .../api/parameter/package-info.md | 5 +++++ .../graph_builder/issues/package-info.java | 7 ------ .../graph_builder/issues/package-info.md | 6 +++++ .../netex/loader/parser/package-info.java | 2 -- .../transferoptimization/package-info.java | 4 ---- .../visualizer/package-info.java | 6 ----- .../visualizer/package-info.md | 5 +++++ ...Testdocument.pdf => MMRI_Testdocument.pdf} | Bin .../opentripplanner/mmri/package-info.java | 21 ------------------ .../org/opentripplanner/mmri/package-info.md | 19 ++++++++++++++++ 15 files changed, 45 insertions(+), 59 deletions(-) delete mode 100644 src/ext/java/org/opentripplanner/ext/datastore/gs/package-info.java create mode 100644 src/ext/java/org/opentripplanner/ext/datastore/gs/package-info.md delete mode 100644 src/ext/java/org/opentripplanner/ext/restapi/resources/package-info.java create mode 100644 src/ext/java/org/opentripplanner/ext/restapi/resources/package-info.md delete mode 100644 src/main/java/org/opentripplanner/api/parameter/package-info.java create mode 100644 src/main/java/org/opentripplanner/api/parameter/package-info.md delete mode 100644 src/main/java/org/opentripplanner/graph_builder/issues/package-info.java create mode 100644 src/main/java/org/opentripplanner/graph_builder/issues/package-info.md delete mode 100644 src/main/java/org/opentripplanner/netex/loader/parser/package-info.java delete mode 100644 src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/package-info.java delete mode 100644 src/main/java/org/opentripplanner/visualizer/package-info.java create mode 100644 src/main/java/org/opentripplanner/visualizer/package-info.md rename src/test/java/org/opentripplanner/mmri/{MMRI Testdocument.pdf => MMRI_Testdocument.pdf} (100%) delete mode 100644 src/test/java/org/opentripplanner/mmri/package-info.java create mode 100644 src/test/java/org/opentripplanner/mmri/package-info.md diff --git a/src/ext/java/org/opentripplanner/ext/datastore/gs/package-info.java b/src/ext/java/org/opentripplanner/ext/datastore/gs/package-info.java deleted file mode 100644 index 82ebac6c832..00000000000 --- a/src/ext/java/org/opentripplanner/ext/datastore/gs/package-info.java +++ /dev/null @@ -1,8 +0,0 @@ -/** - * Add support for Google Cloud Storage, getting all input files and storing the graph.obj in the - * cloud. - *

- * This implementation will use the existing {@link org.opentripplanner.standalone.config.OtpConfigLoader} - * to load config from the local disk. - */ -package org.opentripplanner.ext.datastore.gs; diff --git a/src/ext/java/org/opentripplanner/ext/datastore/gs/package-info.md b/src/ext/java/org/opentripplanner/ext/datastore/gs/package-info.md new file mode 100644 index 00000000000..22ae44bb3e3 --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/datastore/gs/package-info.md @@ -0,0 +1,6 @@ +# Google cloud storage integration + +Add support for Google Cloud Storage, getting all input files and storing the graph.obj in the +cloud. + +This implementation will use the existing OtpConfigLoader to load config from the local disk. diff --git a/src/ext/java/org/opentripplanner/ext/restapi/resources/package-info.java b/src/ext/java/org/opentripplanner/ext/restapi/resources/package-info.java deleted file mode 100644 index 7b66666ee5d..00000000000 --- a/src/ext/java/org/opentripplanner/ext/restapi/resources/package-info.java +++ /dev/null @@ -1,5 +0,0 @@ -/** - * This package contains the JAX-RS-annotated REST resource classes for the OpenTripPlanner public - * API, i.e. the Jersey REST endpoints. - */ -package org.opentripplanner.ext.restapi.resources; diff --git a/src/ext/java/org/opentripplanner/ext/restapi/resources/package-info.md b/src/ext/java/org/opentripplanner/ext/restapi/resources/package-info.md new file mode 100644 index 00000000000..a4bb7eb6f4f --- /dev/null +++ b/src/ext/java/org/opentripplanner/ext/restapi/resources/package-info.md @@ -0,0 +1,4 @@ +# JAX-RS-annotated REST resource classes + +This package contains the JAX-RS-annotated REST resource classes for the OpenTripPlanner public +API, i.e. the Jersey REST endpoints. diff --git a/src/main/java/org/opentripplanner/api/parameter/package-info.java b/src/main/java/org/opentripplanner/api/parameter/package-info.java deleted file mode 100644 index ed9df0c1f61..00000000000 --- a/src/main/java/org/opentripplanner/api/parameter/package-info.java +++ /dev/null @@ -1,6 +0,0 @@ -/** - * This package contains classes which interpret incoming HTTP query parameters. Query parameters - * arrive as Strings, and Jersey will automatically call constructors with a single String - * argument. - */ -package org.opentripplanner.api.parameter; diff --git a/src/main/java/org/opentripplanner/api/parameter/package-info.md b/src/main/java/org/opentripplanner/api/parameter/package-info.md new file mode 100644 index 00000000000..75c928773f9 --- /dev/null +++ b/src/main/java/org/opentripplanner/api/parameter/package-info.md @@ -0,0 +1,5 @@ +# HTTP query parameters + +This package contains classes which interpret incoming HTTP query parameters. Query parameters +arrive as Strings, and Jersey will automatically call constructors with a single String +argument. diff --git a/src/main/java/org/opentripplanner/graph_builder/issues/package-info.java b/src/main/java/org/opentripplanner/graph_builder/issues/package-info.java deleted file mode 100644 index db899fdd206..00000000000 --- a/src/main/java/org/opentripplanner/graph_builder/issues/package-info.java +++ /dev/null @@ -1,7 +0,0 @@ -/** - * Graph builder data import issues represent errors or exceptional conditions encountered during - * the graph building process. They contain descriptive messages and potentially references to the - * objects in the graph that they annotate which facilitate visualization and cataloging/mapping of - * problems. - */ -package org.opentripplanner.graph_builder.issues; diff --git a/src/main/java/org/opentripplanner/graph_builder/issues/package-info.md b/src/main/java/org/opentripplanner/graph_builder/issues/package-info.md new file mode 100644 index 00000000000..424605d5240 --- /dev/null +++ b/src/main/java/org/opentripplanner/graph_builder/issues/package-info.md @@ -0,0 +1,6 @@ +# Graph building issues + +Graph builder data import issues represent errors or exceptional conditions encountered during +the graph building process. They contain descriptive messages and potentially references to the +objects in the graph that they annotate which facilitate visualization and cataloging/mapping of +problems. diff --git a/src/main/java/org/opentripplanner/netex/loader/parser/package-info.java b/src/main/java/org/opentripplanner/netex/loader/parser/package-info.java deleted file mode 100644 index a0b9cd00721..00000000000 --- a/src/main/java/org/opentripplanner/netex/loader/parser/package-info.java +++ /dev/null @@ -1,2 +0,0 @@ -package org.opentripplanner.netex.loader.parser; -// TODO OTP2 - This package need Unit tests diff --git a/src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/package-info.java b/src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/package-info.java deleted file mode 100644 index 22163201b6e..00000000000 --- a/src/main/java/org/opentripplanner/routing/algorithm/transferoptimization/package-info.java +++ /dev/null @@ -1,4 +0,0 @@ -/** - * Package documentation - */ -package org.opentripplanner.routing.algorithm.transferoptimization; diff --git a/src/main/java/org/opentripplanner/visualizer/package-info.java b/src/main/java/org/opentripplanner/visualizer/package-info.java deleted file mode 100644 index e189c27956f..00000000000 --- a/src/main/java/org/opentripplanner/visualizer/package-info.java +++ /dev/null @@ -1,6 +0,0 @@ -/** - * This package contains classes used for visualizing OpenTripPlanner graphs. This graph visualizer - * is intended for debugging purposes and may therefore have arcane developer-oriented features and - * grow new UI components as needed. - */ -package org.opentripplanner.visualizer; diff --git a/src/main/java/org/opentripplanner/visualizer/package-info.md b/src/main/java/org/opentripplanner/visualizer/package-info.md new file mode 100644 index 00000000000..f1b60dde32f --- /dev/null +++ b/src/main/java/org/opentripplanner/visualizer/package-info.md @@ -0,0 +1,5 @@ +# Graph visualizer + +This package contains classes used for visualizing OpenTripPlanner graphs. This graph visualizer +is intended for debugging purposes and may therefore have arcane developer-oriented features and +grow new UI components as needed. diff --git a/src/test/java/org/opentripplanner/mmri/MMRI Testdocument.pdf b/src/test/java/org/opentripplanner/mmri/MMRI_Testdocument.pdf similarity index 100% rename from src/test/java/org/opentripplanner/mmri/MMRI Testdocument.pdf rename to src/test/java/org/opentripplanner/mmri/MMRI_Testdocument.pdf diff --git a/src/test/java/org/opentripplanner/mmri/package-info.java b/src/test/java/org/opentripplanner/mmri/package-info.java deleted file mode 100644 index 563a92abdc5..00000000000 --- a/src/test/java/org/opentripplanner/mmri/package-info.java +++ /dev/null @@ -1,21 +0,0 @@ -/** - * What is this package doing here? - *

- * In 2013, significant improvements were made to OTP as part of a precommercial procurement project - * in The Netherlands called MMRI ("MultiModale ReisInformatie" => "multimodal travel information"). - * This project is itself part of a larger project called "Better Benutten" => "better utilization". - * Most effort concentrated on the implementation of GTFS-RT updates and related improvements to the - * architecture of OTP. Additionally, a testing module was developed to verify that all the planners - * that were involved in the project (not just OTP) met a minimum set of requirements. OTP was first - * to pass all tests, ahead of two different solutions. Unfortunately, having two sets of tests does - * not make it simpler to continuously verify that OTP still functions correctly, which is why these - * MMRI tests have now been added to OTP's own test suite. These versions are intended to be a close - * approximation of reality, but several minor shortcuts have been taken, like applying trip updates - * directly to the graph instead of going through the thread-safe graph writer framework. Given that - * thread-safety is a technical issue and not a functional one, this is considered to be - * acceptable. - *

- * The test cases are described here. and in - * here - */ -package org.opentripplanner.mmri; diff --git a/src/test/java/org/opentripplanner/mmri/package-info.md b/src/test/java/org/opentripplanner/mmri/package-info.md new file mode 100644 index 00000000000..5d4f541f1cb --- /dev/null +++ b/src/test/java/org/opentripplanner/mmri/package-info.md @@ -0,0 +1,19 @@ +# MMRI ("MultiModale ReisInformatie" => "multimodal travel information") + +What is this package doing here? + +In 2013, significant improvements were made to OTP as part of a precommercial procurement project +in The Netherlands called MMRI ("MultiModale ReisInformatie" => "multimodal travel information"). +This project is itself part of a larger project called "Better Benutten" => "better utilization". +Most effort concentrated on the implementation of GTFS-RT updates and related improvements to the +architecture of OTP. Additionally, a testing module was developed to verify that all the planners +that were involved in the project (not just OTP) met a minimum set of requirements. OTP was first +to pass all tests, ahead of two different solutions. Unfortunately, having two sets of tests does +not make it simpler to continuously verify that OTP still functions correctly, which is why these +MMRI tests have now been added to OTP's own test suite. These versions are intended to be a close +approximation of reality, but several minor shortcuts have been taken, like applying trip updates +directly to the graph instead of going through the thread-safe graph writer framework. Given that +thread-safety is a technical issue and not a functional one, this is considered to be +acceptable. + +The test cases are described [here](https://github.com/plannerstack/testset) and [here](./MMRI_Testdocument.pdf) From a6aa0ae6ef74a3855c3a922d8a2cb8e3c4a8d7a5 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Fri, 11 Oct 2024 16:35:20 +0300 Subject: [PATCH 7/9] Generate package-info.java files with a template that uses @ParametersAreNonnullByDefault --- .gitignore | 1 + pom.xml | 24 +++++++++++++++++++ .../templates/package-info-template.java | 4 ++++ 3 files changed, 29 insertions(+) create mode 100644 src/build/templates/package-info-template.java diff --git a/.gitignore b/.gitignore index 6fac28d2178..3227500244d 100644 --- a/.gitignore +++ b/.gitignore @@ -29,6 +29,7 @@ o_o_standalone_config_IncludeFileDirectiveTest_part.json .venv/ _site/ build/ +!/src/build/ dist/ doc/user/_build/ gen-java/ diff --git a/pom.xml b/pom.xml index 1f5bb3ed8a3..1ad24efdb9f 100644 --- a/pom.xml +++ b/pom.xml @@ -515,6 +515,30 @@ com.google.protobuf:protoc:3.22.0:exe:${os.detected.classifier} + + + com.github.bohnman + package-info-maven-plugin + 1.1.0 + + + ${project.basedir}/src/main/java + ${project.basedir}/target/generated-sources + + + ** + + + + + + + + generate + + + + diff --git a/src/build/templates/package-info-template.java b/src/build/templates/package-info-template.java new file mode 100644 index 00000000000..7cbb693f540 --- /dev/null +++ b/src/build/templates/package-info-template.java @@ -0,0 +1,4 @@ +@ParametersAreNonnullByDefault +package org.opentripplanner; + +import javax.annotation.ParametersAreNonnullByDefault; From 95f1d445f074dd867bd275c4a9e6d8311f678829 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Thu, 17 Oct 2024 14:42:04 +0300 Subject: [PATCH 8/9] Move package-info-template to new src location --- .gitignore | 2 +- .../src}/build/templates/package-info-template.java | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename {src => application/src}/build/templates/package-info-template.java (100%) diff --git a/.gitignore b/.gitignore index 3227500244d..d67b620f7b1 100644 --- a/.gitignore +++ b/.gitignore @@ -29,7 +29,7 @@ o_o_standalone_config_IncludeFileDirectiveTest_part.json .venv/ _site/ build/ -!/src/build/ +!/application/src/build/ dist/ doc/user/_build/ gen-java/ diff --git a/src/build/templates/package-info-template.java b/application/src/build/templates/package-info-template.java similarity index 100% rename from src/build/templates/package-info-template.java rename to application/src/build/templates/package-info-template.java From c7e16bc0f6f5ae27f2ed896b589a31cedb81e901 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Thu, 17 Oct 2024 14:42:46 +0300 Subject: [PATCH 9/9] Move package info plugin to application pom --- application/pom.xml | 23 +++++++++++++++++++++++ pom.xml | 24 ------------------------ 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/application/pom.xml b/application/pom.xml index 49c738a251e..80446348801 100644 --- a/application/pom.xml +++ b/application/pom.xml @@ -513,6 +513,29 @@ + + com.github.bohnman + package-info-maven-plugin + 1.1.0 + + + ${project.basedir}/src/main/java + ${project.basedir}/target/generated-sources + + + ** + + + + + + + + generate + + + + diff --git a/pom.xml b/pom.xml index bfe9c600cc5..0b4dd2ea10c 100644 --- a/pom.xml +++ b/pom.xml @@ -378,30 +378,6 @@ com.google.protobuf:protoc:3.22.0:exe:${os.detected.classifier} - - - com.github.bohnman - package-info-maven-plugin - 1.1.0 - - - ${project.basedir}/src/main/java - ${project.basedir}/target/generated-sources - - - ** - - - - - - - - generate - - - -