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

Avoid null ksp_version in Netkan #2558

Merged
merged 1 commit into from
Oct 31, 2018

Conversation

HebaruSan
Copy link
Member

Problem

#2553 tried to fix a problem with setting "ksp_version": "any" in a netkan that also has a $vref. The change worked insofar as netkan was able to generate a .ckan file instead of erroring out, but the script that checks the .ckan file against the schema found a problem: ksp_version was null, and that's not allowed.

    "ksp_version": null,
Validating built/CommunityTerrainTexturePack-1-1.0.4.ckan.. Failed! See below for error description.
None is not of type u'string'

Failed validating u'type' in schema[u'properties'][u'ksp_version']:
    {u'description': u'A version of KSP',
     u'pattern': u'^(any|[0-9]+\\.[0-9]+(\\.[0-9]+)?)$',
     u'type': u'string'}

On instance[u'ksp_version']:
    None

This is blocking KSP-CKAN/NetKAN#6791.

Cause

KspVersion.Any.ToString() returns null because of this:

return s.Equals(string.Empty) ? null : s;

(This is kind of gross, since the string "any" is specifically allowed in the spec and could be used here, but I'm reluctant to mess with such a core assumption.)

Then when we parse "any" and return KspVersion.Any, these lines then set ksp_version to null:

if (kspMin != null && kspMax != null)
{
// If we have both a minimum and maximum KSP version...
if (kspMin.CompareTo(kspMax) == 0)
{
// ...and they are equal, then just set ksp_version
json["ksp_version"] = kspMin.ToString();
}

Changes

The if checks in these lines are in charge of keeping null out of the lists of possible min and max versions:

if (existingKspMin != null)
kspMins.Add(existingKspMin);
if (avcKspMin != null)
kspMins.Add(avcKspMin);
if (existingKspMax != null)
kspMaxes.Add(existingKspMax);
if (avcKspMax != null)
kspMaxes.Add(avcKspMax);

Now they're updated to also prevent KspVersion.Any from being added to those lists. This is done with a new static helper function KspVersion.IsNullOrAny in the style of string.IsNullOrEmpty. This will cause the AVC transformer to not make any changes, and the "any" value from the original netkan will flow through to the .ckan file, which is valid according to the spec.

@HebaruSan HebaruSan added Bug Pull request Netkan Issues affecting the netkan data labels Oct 28, 2018
@politas politas merged commit 1ffdab2 into KSP-CKAN:master Oct 31, 2018
politas added a commit that referenced this pull request Oct 31, 2018
@HebaruSan HebaruSan deleted the fix/netkan-null-version branch October 31, 2018 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Netkan Issues affecting the netkan data Pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants