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

Fare rule entities -- internal representation #72

Merged
merged 124 commits into from Jun 15, 2020
Merged

Conversation

lionel-nj
Copy link
Contributor

@lionel-nj lionel-nj commented Mar 20, 2020

Summary:

This PR relates to processing of entities from fare_rules.txt. The goal is to use the required use cases implemented via PR #44 that allow to go from a ParsedEntity representing a row from fare_rules.txt to a specific concrete internal representation.

See #44

Expected behavior:

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with gradle test to make sure you didn't break anything
  • Format the title like "Fix #<issue_number> - " (for example - "Fix Bug: Running using documented Docker documentation fails #1111 - Check for null value before using field")
  • Linked all relevant issues
  • Include screenshot(s) showing how this pull request works and fixes the issue(s)

lionel-nj and others added 8 commits Mar 12, 2020
-implement new class Agency representing an agency defined as detailed by https://gtfs.org/reference/static#agencytxt
-define getters
-implement builder pattern for immutable object construction
-modify gradle.build to allow use of annotations
-modify AgencyBuilder access modifiers from protected to private
-change variables and fields names to a java camel case formatting
-improve code legibility
-adapt annotations for relevant fields
-adjust camel case formatting
-add constructor to AgencyBuilder for required fields: agencyName, agencyUrl, agencyTimezone to forbid the the possibility to construct an Agency object without the required fields
-delete builder setters for agencyName, agencyUrl, agencyTimezone since these attributes are defined via the constructor
-delete comments
-improve code legibility
…dencies

-move "implementation 'org.jetbrains:annotations:19.0.0'" before test dependencies
- implement new class FareRule representing an agency defined as detailed by https://gtfs.org/reference/static#fare_rulestxt
- implement getters
- implement builder pattern for immutable object construction
- add dependency to build.gradle to allow annotation use
@lionel-nj lionel-nj requested a review from a user Mar 20, 2020
@lionel-nj lionel-nj self-assigned this Mar 20, 2020
lionel-nj and others added 4 commits Mar 23, 2020
-make Agency class constructor private to enforce builder use
-finalize methods parameters to avoid probable bugs and ease future use in multi
-delete empty line
-add setters for required fields in AgencyBuilder
-add setters for required fields in FareRuleBuilder
-remove empty line from gradle.build file
-finalize methods parameters to avoid potential future bugs
@lionel-nj lionel-nj requested a review from a user Mar 23, 2020
lionel-nj and others added 8 commits Mar 24, 2020
-add @nullable and @NotNull annotations in Agency class
-add @nullable and @NotNull annotations to fields and variables
-move Agency class to use case module
-add dependency on mockito for tests
-add dependency in build.gradle to enable usage of annotations
-implement use case ProcessParsedAgency
-create interface GtfsDataRepository
-implement said interface in usecase module
-implement new abstract class Entity for all Gtfs entities: agencies, stops, etc...
-add method addEntity to be able to add said entities to a GtfsDataRepository
-implement tests
-add new module "hasher" for future file hashing
-add addEntity method to GtfsDataRepository interface
-add build.gradle for future hasher
-delete hasher related file: this feature will be treated in another PR
-move back entity to domain layer
-remove visitor pattern: useless
-rename 'entity' subpackage in domain layer as gtfsentity for clarity
-correct dependency on mockito in build.gradle to avoid instability
-add getAgencyById method to GtfsDataRepository interface and its implementation in InMemoryGtfsDataRepository
-remove hasher related content in setting.gradle: this feature will be treated in another PR
-move curly bracket to a new line in build.gradle (domain layer)
-delete duplicate dependency in build.gradle (adapter layer)
lionel-nj added 5 commits Mar 31, 2020
…tion

-rename method getAgencyList to getAgencyCollection
-change type of agency container to Hashmap<String, Agency> to ease future search in the Gtfs data repository
-rename test for clarity
-adapt tests to these changes
-add test for Agency to test Agency.AgencyBuilder build method behavior
-remove AgencyBuilder constructor to enable injection in ProcessParsedAgency usecase constructor
-inject builder in ProcessParsedAgency usecase constructor
-make use case execution method throw NullPointerException
-adapt and implement new tests for ProcessParsedAgency
-add licence header
-add javadoc comments
-delete useless dependency in build.gradle files
-add GtfsDataRepository interface to add GtfsEntities
-concrete implement of said interface
-add test class to GtfsDataRepository implementation
-add abstract class GtfsEntity: all entities will inherit from this class
-enable Junit5 in build.gradle for tests
-change package name gtfsentity to gtfs
-change package name gtfsentity to gtfs
lionel-nj added 5 commits May 7, 2020
# Conflicts:
#	domain/src/main/java/org/mobilitydata/gtfsvalidator/domain/entity/gtfs/Agency.java
#	domain/src/main/java/org/mobilitydata/gtfsvalidator/domain/entity/gtfs/routes/Route.java
#	domain/src/test/java/org/mobilitydata/gtfsvalidator/domain/entity/gtfs/AgencyTest.java
#	domain/src/test/java/org/mobilitydata/gtfsvalidator/domain/entity/gtfs/RouteTest.java
#	usecase/src/main/java/org/mobilitydata/gtfsvalidator/usecase/ProcessParsedRoute.java
…ethod

- make FareRule extend GtfsEntity
- remove constructor of entity builder
- make noticeCollection a private member of class FareRuleBuilder for future
- make builder build method return EntityBuildResult object
- write tests
…m/to GtfsDataRepository

- implement method addFareRule to add entity to data repo
- implement method getFareRule to retrieve entity from data repo
- write test to verify said methods behavior
- adapt GtfsDataRepository interface to said changes
- implement use case ProcessParsedFareRule to go from a parsed entity representing a row from fare_rules.txt to an internal representation using FareRule objects
- write test to verify said use case execution method testing:
  - invalid parsed entity generates notices an is not added to data repo
  - valid parsed entity does not generate notice and is added to data repo
  - duplicate parsed entity generates notice and is not added to data repo
@lionel-nj lionel-nj changed the base branch from implement-gtfs-data-repository to master May 7, 2020
@lionel-nj lionel-nj marked this pull request as ready for review Jun 2, 2020
lionel-nj added 3 commits Jun 3, 2020
# Conflicts:
#	adapter/repository/in-memory-simple/src/main/java/org/mobilitydata/gtfsvalidator/db/InMemoryGtfsDataRepository.java
#	adapter/repository/in-memory-simple/src/test/java/org/mobilitydata/gtfsvalidator/db/InMemoryGtfsDataRepositoryTest.java
#	application/cli-app/src/main/java/org/mobilitydata/gtfsvalidator/Main.java
#	config/src/main/java/org/mobilitydata/gtfsvalidator/config/DefaultConfig.java
#	usecase/src/main/java/org/mobilitydata/gtfsvalidator/usecase/port/GtfsDataRepository.java
…ions

- add comments to detail the use of annotations to suppress warnings
Copy link

@ghost ghost left a comment

Key generation code is duplicated. Inline remarks contain a suggestion as to how to fix that, which is open to discussion.

lionel-nj added 2 commits Jun 11, 2020
- use static method to retrieve key from FareRule entity
- finalize variable
- remove semi-colon from the definition of entity key
- adapt tests
- add comment for code clarity
- modify javadoc
# Conflicts:
#	adapter/repository/in-memory-simple/src/main/java/org/mobilitydata/gtfsvalidator/db/InMemoryGtfsDataRepository.java
#	adapter/repository/in-memory-simple/src/test/java/org/mobilitydata/gtfsvalidator/db/InMemoryGtfsDataRepositoryTest.java
#	application/cli-app/src/main/java/org/mobilitydata/gtfsvalidator/Main.java
#	config/src/main/java/org/mobilitydata/gtfsvalidator/config/DefaultConfig.java
#	usecase/src/main/java/org/mobilitydata/gtfsvalidator/usecase/port/GtfsDataRepository.java
@lionel-nj lionel-nj requested a review from a user Jun 11, 2020
ghost
ghost approved these changes Jun 15, 2020
Copy link

@ghost ghost left a comment

lgtm

#285 created

@ghost ghost mentioned this pull request Jun 15, 2020
1 task
@lionel-nj lionel-nj merged commit b986569 into master Jun 15, 2020
1 check passed
@lionel-nj lionel-nj deleted the fare-rules-class branch Jun 15, 2020
ghost pushed a commit that referenced this pull request Jun 15, 2020
@ghost ghost linked an issue Jul 21, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REQUEST] Clarify FareRule class API
1 participant