Remove the null=True on CharField fields of the PackageURLMixin #36

[tdruez]

From https://docs.djangoproject.com/en/3.0/ref/models/fields/#null

Avoid using null on string-based fields such as CharField and TextField. If a string-based field has null=True, that means it has two possible values for “no data”: NULL, and the empty string. In most cases, it’s redundant to have two possible values for “no data;” the Django convention is to use the empty string, not NULL.

This is an issue when adding database constraints based on those fields as '' and null values do not behave the same.

Add a empty=None param on the PackageURL.to_dict method to allow custom value like empty string '' in place of None

Signed-off-by: Thomas Druez tdruez@nexb.com

[pombredanne]

@tdruez Thank you ++... it makes a lot of sense. I have only one minor nitpicking for your consideration.
@sbs2001 ping... does this make sense for you too since this is used in VulnerableCode ?

[pombredanne]

Have you considered setattr(self, field_name, value or '')?

[tdruez]

Yes I did. It was my initial approach, but the empty value is now controlled in the above to_dict which makes it unneeded in the setattr.

[pombredanne]

Why using only there?

[tdruez]

Could you rephrase your question?

[pombredanne]

Sorry for the poor wording ... I mean why would you only translate the empty values to empty if empty is not None?
Why not doing it all the time and therefore removing the test if empty is not None:?

[pombredanne]

Since data is already an OrderedDict... you may want to modify it in place but this is minor:

for k, v in data.items():
    data[k] = v or empty

[tdruez]

Changed in f2e962f

[sbs2001]

@pombredanne This makes total sense and is definitely needed. It will prevent aboutcode-org/vulnerablecode#206 (comment) from happening.

Thanks @tdruez

[tdruez]

@sbs2001 Thanks for your input 👍