search

CCI-MOC / flocx-market

2 stars 9 forks source link

Input validation on server_config #141

Open ljmcgann opened 5 years ago

ljmcgann commented 5 years ago

Addresses github issues 118 and 99 by checking that the types for a post request are correct. TG-177

codecov-io commented 5 years ago

Codecov Report

Merging #141 into master will increase coverage by 0.03%. The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #141      +/-   ##
==========================================
+ Coverage    92.9%   92.93%   +0.03%     
==========================================
  Files          52       52              
  Lines        2114     2138      +24     
==========================================
+ Hits         1964     1987      +23     
- Misses        150      151       +1
Impacted Files Coverage Δ
flocx_market/tests/unit/api/test_app_bid.py 100% <ø> (ø) :arrow_up:
flocx_market/tests/unit/api/test_app_offer.py 100% <ø> (ø) :arrow_up:
flocx_market/db/sqlalchemy/api.py 93.93% <ø> (ø) :arrow_up:
flocx_market/api/bid.py 91.89% <0%> (ø) :arrow_up:
flocx_market/api/offer.py 91.89% <0%> (ø) :arrow_up:
flocx_market/tests/unit/db/sqlalchemy/test_api.py 96.6% <100%> (+0.08%) :arrow_up:
flocx_market/common/exception.py 100% <100%> (ø) :arrow_up:
flocx_market/db/sqlalchemy/models.py 97.43% <84.61%> (-1.08%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 736af76...8f051f9. Read the comment docs.

ljmcgann commented 5 years ago

Is there some kind of secret to this? I tried to make a function following the same pattern as Lar's cost code, and I get an error ERROR oslo_db.sqlalchemy.exc_filters sqlite3.IntegrityError: NOT NULL constraint failed: bids.server_config_query Everytime. The function I make can see the value just fine but this error still gets thrown. I can change the model to make nullable true to pass but this leads to weird errors in the matcher

larsks commented 5 years ago

@ljmcgann does your verifier function return a value?

ljmcgann commented 5 years ago

That was it

ljmcgann commented 5 years ago

Do we want the serv_config for an offer to also be a dictionary and have a key 'properties'?

ljmcgann commented 5 years ago

I knew I was gonna get called out on the InvalidInput exception but as Mainn said, I didn't want to catch all ValueErrors just to throw another error with the code and message. Also that just seems worse as if a ValueError gets thrown in some unexpected part of the code we want to know about it. We know all the places where InvalidInput can be thrown so that behaviour isn't unexpected.