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

Initial implementation of database and model field checker. #3452

Merged
merged 3 commits into from Feb 7, 2020

Conversation

@jim
Copy link
Contributor

jim commented Feb 3, 2020

Description

We've now had at least two instances of deployed systems breaking because the application attempted to query a model that contained a NULL value in an incompatible column. This has also cropped up in non-deployed systems during automated performance testing.

As a reminder, Pop requires that struct fields that map to nullable database columns be defined either as pointers or using its custom null types. Otherwise, there will be scan errors such as the one addressed by #3439.

As a followup to that PR, I did a little manual checking of models. There were enough mismatches that I thought it would be good to have an automated way to keep an eye on this.

Reviewer Notes

This PR adds a tool that will check that our model struct definitions and database tables are in alignment. It should probably get wrapped in a make target, but I wanted to get some early feedback before going too far.

Eventually we might want to add this to CI, but we'll have to contend with the existing issues first.

Setup

$ go run cmd/compare-db-to-models/main.go

You should see something like this:

AccessCode
  *MoveType : move_type is NOT NULL

ClientCert
  Sha256Digest : sha256_digest is NULL
  Subject : subject is NULL
  AllowDpsAuthAPI : allow_dps_auth_api is NULL
  AllowOrdersAPI : allow_orders_api is NULL

Customer
  Agency : agency is NULL
  FirstName : first_name is NULL
  LastName : last_name is NULL
  UserID : user_id is NULL

Move
  Locator : locator is NULL
  *Show : show is NOT NULL

MoveTaskOrder
  MoveOrderID : move_order_id is NULL

MovingExpenseDocument
  RequestedAmountCents : requested_amount_cents is NULL
  PaymentMethod : payment_method is NULL
  ReceiptMissing : receipt_missing is NULL

Notification
  ServiceMemberID : service_member_id is NULL

PaymentServiceItem
  ServiceItemID : service_item_id is NULL

Tariff400ngFullPackRate
  CreatedAt : created_at is NULL
  UpdatedAt : updated_at is NULL
  Schedule : schedule is NULL
  WeightLbsLower : weight_lbs_lower is NULL
  WeightLbsUpper : weight_lbs_upper is NULL
  RateCents : rate_cents is NULL
  EffectiveDateLower : effective_date_lower is NULL
  EffectiveDateUpper : effective_date_upper is NULL

Tariff400ngFullUnpackRate
  CreatedAt : created_at is NULL
  UpdatedAt : updated_at is NULL
  Schedule : schedule is NULL
  RateMillicents : rate_millicents is NULL
  EffectiveDateLower : effective_date_lower is NULL
  EffectiveDateUpper : effective_date_upper is NULL

Tariff400ngLinehaulRate
  CreatedAt : created_at is NULL
  UpdatedAt : updated_at is NULL
  DistanceMilesLower : distance_miles_lower is NULL
  DistanceMilesUpper : distance_miles_upper is NULL
  Type : type is NULL
  WeightLbsLower : weight_lbs_lower is NULL
  WeightLbsUpper : weight_lbs_upper is NULL
  RateCents : rate_cents is NULL
  EffectiveDateLower : effective_date_lower is NULL
  EffectiveDateUpper : effective_date_upper is NULL

Tariff400ngServiceArea
  CreatedAt : created_at is NULL
  UpdatedAt : updated_at is NULL
  Name : name is NULL
  ServiceArea : service_area is NULL
  ServicesSchedule : services_schedule is NULL
  LinehaulFactor : linehaul_factor is NULL
  ServiceChargeCents : service_charge_cents is NULL
  EffectiveDateLower : effective_date_lower is NULL
  EffectiveDateUpper : effective_date_upper is NULL
  SIT185ARateCents : sit_185a_rate_cents is NULL
  SIT185BRateCents : sit_185b_rate_cents is NULL
  SITPDSchedule : sit_pd_schedule is NULL

Tariff400ngShorthaulRate
  CreatedAt : created_at is NULL
  UpdatedAt : updated_at is NULL
  CwtMilesLower : cwt_miles_lower is NULL
  CwtMilesUpper : cwt_miles_upper is NULL
  RateCents : rate_cents is NULL
  EffectiveDateLower : effective_date_lower is NULL
  EffectiveDateUpper : effective_date_upper is NULL

Tariff400ngZip3
  CreatedAt : created_at is NULL
  UpdatedAt : updated_at is NULL
  Zip3 : zip3 is NULL
  BasepointCity : basepoint_city is NULL
  State : state is NULL
  ServiceArea : service_area is NULL
  RateArea : rate_area is NULL
  Region : region is NULL

Tariff400ngZip5RateArea
  CreatedAt : created_at is NULL
  UpdatedAt : updated_at is NULL
  RateArea : rate_area is NULL
  Zip5 : zip5 is NULL

Upload
  StorageKey : storage_key is NULL

WeightTicketSetDocument
  MoveDocumentID : move_document_id is NULL
  EmptyWeightTicketMissing : empty_weight_ticket_missing is NULL
  FullWeightTicketMissing : full_weight_ticket_missing is NULL
  VehicleNickname : vehicle_nickname is NULL
  WeightTicketSetType : weight_ticket_set_type is NULL
  TrailerOwnershipMissing : trailer_ownership_missing is NULL
exit status 1

Code Review Verification Steps

  • Code follows the guidelines for Logging
  • The requirements listed in
    Querying the Database Safely
    have been satisfied.
  • Request review from a member of a different team.
  • Have the Jira acceptance criteria been met for this change?
@jim jim requested review from chrisgilmerproj, chrisrcoles and tinyels Feb 3, 2020
@jim jim added the roci label Feb 3, 2020
@jim jim requested review from garrettqmartin8, LeDeep, lynzt and reggieriser Feb 3, 2020
@chrisgilmerproj

This comment has been minimized.

Copy link
Contributor

chrisgilmerproj commented Feb 3, 2020

Is the output the list of all errors (mistmatches) between the code and DB?

@jim

This comment has been minimized.

Copy link
Contributor Author

jim commented Feb 3, 2020

Copy link
Contributor

garrettqmartin8 left a comment

Looks great! Works well when testing by fixing a few instances listed in the output.

Copy link
Contributor

chrisgilmerproj left a comment

I approve of this. Would love to see it have a Makefile target and then we can talk about putting this in CircleCI in a separate PR.

@lynzt
lynzt approved these changes Feb 5, 2020
Copy link
Contributor

lynzt left a comment

This is a really helpful script... nice!!

Copy link
Contributor

chrisgilmerproj left a comment

🚀

@jim jim merged commit d546aa8 into master Feb 7, 2020
15 checks passed
15 checks passed
ci/circleci: acceptance_tests_experimental Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_local Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_staging Your tests passed on CircleCI!
Details
ci/circleci: build_app Your tests passed on CircleCI!
Details
ci/circleci: build_migrations Your tests passed on CircleCI!
Details
ci/circleci: build_storybook_app Your tests passed on CircleCI!
Details
ci/circleci: build_tasks Your tests passed on CircleCI!
Details
ci/circleci: build_tools Your tests passed on CircleCI!
Details
ci/circleci: check_generated_code Your tests passed on CircleCI!
Details
ci/circleci: client_test Your tests passed on CircleCI!
Details
ci/circleci: integration_tests Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_golang Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_yarn Your tests passed on CircleCI!
Details
ci/circleci: pre_test Your tests passed on CircleCI!
Details
ci/circleci: server_test Your tests passed on CircleCI!
Details
@jim jim deleted the jb-compare-db-to-models branch Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.