Skip to content
This repository has been archived by the owner on Nov 17, 2022. It is now read-only.

Modify privacy level of information within a Ruleset #246

Merged
merged 9 commits into from
Nov 20, 2017

Conversation

hayfield
Copy link
Contributor

@hayfield hayfield commented Nov 16, 2017

A function and variable were public properties of a Ruleset when they didn't need to be.

The variable was something that could become inconsistent with the rules attribute.
The function acted on this variable in a manner that shouldn't ever need doing outside Ruleset initialisation.

This also documents the attribute that a Ruleset has.

This came about in the writing up of #245 to determine the information required to determine Ruleset equality.

The function performs an internal check that a string is valid before attempting to parse it.
There is no reasonable manner that it could be used publically. As such, it makes sense for it to be private.
This is a value used during the initial parsing of a ruleset_str.
Rules can be added to Ruleset.rules separately to this updating, making them inconsistent.

This likely means that it should be a variable rather than an attribute. The interim step is to make it private.
As noted in 52a0360 this can become inconsistent with Ruleset.rules.
There should be a single attribute to maintain state. This ensures there is only one.
2 of the 3 potential errors had error messages. This adds the 3rd.
@hayfield hayfield added the rulesets Relating to IATI Rulesets. label Nov 16, 2017
Information about Ruleset changes
@hayfield hayfield added the complete A PR that is in a state that is ready for review. label Nov 16, 2017
@hayfield hayfield requested a review from a team November 16, 2017 10:17
@dalepotter
Copy link
Contributor

Is there a use case for a user to validate a custom ruleset against the schema? How are we suggesting this is done? Check before using the json string to instantiate a ruleset?

Copy link
Contributor

@dalepotter dalepotter left a comment

Choose a reason for hiding this comment

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

See comment above...

@hayfield
Copy link
Contributor Author

Is there a use case for a user to validate a custom ruleset against the schema? How are we suggesting this is done?

Try and instantiate a Ruleset from a string. If it instantiates all is good.

Validating a string against the Ruleset Schema by calling a function on a random instance of the Ruleset class seems... very odd.

@hayfield
Copy link
Contributor Author

hayfield commented Nov 16, 2017

OR (if you want to do things raw)

import json
import jsonschema
import iati.default
import iati.utilites
json.loads(ruleset_str, object_pairs_hook=iati.utilities.dict_raise_on_duplicates)
jsonschema.validate(self.ruleset, iati.default.ruleset_schema())

@dalepotter
Copy link
Contributor

dalepotter commented Nov 16, 2017

Validating a string against the Ruleset Schema by calling a function on a random instance of the Ruleset class seems... very odd.

Fair enough - so... just testing this: I was expecting a ValueError('The provided Ruleset does not validate against the Ruleset Schema') when I ran this:

>>> import iati
>>> iati.Ruleset('{"json": "test"}')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/dalepotter/Documents/iati.core/iati/rulesets.py", line 87, in __init__
    self._set_rules(ruleset_dict)
  File "/Users/dalepotter/Documents/iati.core/iati/rulesets.py", line 139, in _set_rules
    for rule_type, cases in rule.items():
AttributeError: 'list' object has no attribute 'items'

@hayfield
Copy link
Contributor Author

That looks like a bug!

@hayfield hayfield added incomplete A PR that is in a state that is not ready for review. and removed complete A PR that is in a state that is ready for review. labels Nov 16, 2017
@hayfield
Copy link
Contributor Author

It's a bug with the Ruleset Schema.

It would be expected that a Ruleset is in the format:

{
    "xpath": {
        "dictionary": "of values"
    }
}

As it is, however, the Ruleset Schema does not specify that a Ruleset must be a dictionary of dictionaries. The following is therefore valid:

{
    "xpath": "some random string"
}

This can be fixed by updating patternProperties in the Ruleset Schema to read:

"patternProperties": {
        ".+": {
                "type": "object",
                "properties": {
                        [...]
                }
        }
}

ie. add "type": "object",

IATI/IATI-Rulesets#49 details a problem with the Ruleset Schema.
This works around the problem.
@hayfield hayfield added complete A PR that is in a state that is ready for review. and removed incomplete A PR that is in a state that is not ready for review. labels Nov 16, 2017
@hayfield hayfield requested a review from a team November 16, 2017 14:45
@hayfield hayfield merged commit aa94df3 into dev Nov 20, 2017
@hayfield hayfield deleted the ruleset-attrib-and-privacy branch November 20, 2017 09:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
complete A PR that is in a state that is ready for review. rulesets Relating to IATI Rulesets.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants