HTTP-APIs / hydrus

REST server - Flask Hydra-powered for Semantic Web
https://pypi.org/project/hydrus/
MIT License
195 stars 130 forks source link

Matching done in db_models for datatype needs to be stricter. #595

Closed chrizandr closed 3 years ago

chrizandr commented 3 years ago

I'm submitting a

Current Behaviour:

Right now, the matching for datatype in db_models.py uses a split and just checks if the ending of the URI is integer. This needs to be made stricter so that we only match "https://www.w3.org/TR/xmlschema-2/#integer" or "https://www.w3.org/TR/xmlschema-2/#float". Right now we will also match "https://myserver.com/someendpoint#integer" which need not necessarily point to integer.

Expected Behaviour:

Match strictly only for "https://www.w3.org/TR/xmlschema-2/#integer" or "https://www.w3.org/TR/xmlschema-2/#float". This can be complemented by https://github.com/HTTP-APIs/hydra-python-core/issues/92 so that users do not make errors when adding these URIs in the keyword args.

Steps to reproduce:

Snapshot:

Environment:

* python version * pip version * OS details ### Do you want to work on this issue?
farazkhanfk7 commented 3 years ago

Can't figure out how we are going to match the whole string https://www.w3.org/TR/xmlschema-2/#integer in hydrus . Obviously we can't hardcode these things or get it from context to check it with multiple if conditions. To fix this I think we can make a change in hydra-python-core which will allow users to add range like :

HydraClassProp(prop_uri, prop_title, required=True, read=True, write=True, range="float")  // instead of "xsd:float"

Later we'll add this in supported property's dict as "range": "xsd:float". This can be used to make sure no such URI like "https://myserver.com/someendpoint#integer" can be passed in hydrus because everytime it'll have to pass through doc_maker.create_doc() first and it'll will only allow datatypes that are defined using xsd:

chrizandr commented 3 years ago

Yes this is why I created both issues together. Your approach is what I had in mind as well.

Mec-iS commented 3 years ago

merged in #599