Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify @Nullable and @Nonnull usage [ci skip] #6095

Merged
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ o_o_standalone_config_IncludeFileDirectiveTest_part.json
.venv/
_site/
build/
!/src/build/
t2gran marked this conversation as resolved.
Show resolved Hide resolved
dist/
doc/user/_build/
gen-java/
Expand Down
13 changes: 13 additions & 0 deletions doc/dev/decisionrecords/Codestyle.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,19 @@ 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 they can return null values
- Method parameters should be marked as `@Nullable` if they can take null values.
- On fields:
- 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
`@Nullable` annotation is missing even if it should have been used. Those can be updated
to use the `@Nullable` annotation.

## JavaScript

As of #206, we
Expand Down
24 changes: 24 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,30 @@
<protocArtifact>com.google.protobuf:protoc:3.22.0:exe:${os.detected.classifier}</protocArtifact>
</configuration>
</plugin>

<plugin>
<groupId>com.github.bohnman</groupId>
<artifactId>package-info-maven-plugin</artifactId>
<version>1.1.0</version>
<configuration>
<!-- Couldn't get this to work for main and ext code at the same time. -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry Joel for not looking at this before, but if you want 2 executions with different config, then you create two execution blocks and move the config inside the execution. This is the default, so moving only the sourceDirectory should be enough. This is however good enough.

<sourceDirectory>${project.basedir}/src/main/java</sourceDirectory>
<outputDirectory>${project.basedir}/target/generated-sources</outputDirectory>
<packages>
<package>
<pattern>**</pattern>
<template>${project.basedir}/src/build/templates/package-info-template.java</template>
</package>
</packages>
</configuration>
<executions>
<execution>
<goals>
<goal>generate</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

Expand Down
4 changes: 4 additions & 0 deletions src/build/templates/package-info-template.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
@ParametersAreNonnullByDefault
package org.opentripplanner;

import javax.annotation.ParametersAreNonnullByDefault;

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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.

This file was deleted.

This file was deleted.

5 changes: 2 additions & 3 deletions src/main/java/org/opentripplanner/transit/model/package.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@ 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`,
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.
Expand Down

This file was deleted.

5 changes: 5 additions & 0 deletions src/main/java/org/opentripplanner/visualizer/package-info.md
Original file line number Diff line number Diff line change
@@ -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.
21 changes: 0 additions & 21 deletions src/test/java/org/opentripplanner/mmri/package-info.java

This file was deleted.

19 changes: 19 additions & 0 deletions src/test/java/org/opentripplanner/mmri/package-info.md
Original file line number Diff line number Diff line change
@@ -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)
Loading