Patch to make GetConfigurationValues tolerant of values provided by third party vendors that don't have an Id or Url associated with them

[gmcelhanon]

I signed up for CloudAMQP (RabbitMQ), and was given a config value without an id or url. GetConfigurationValues was throwing a null reference exception because the Url returned with the value from AppHarbor is null. With my change, the API is now tolerant of such missing data, but obviously if I attempt to subsequently retrieve the variable with an Id of 0, I will get a null value back.

If you use your browser and pull https://appharbor.com/applications/YOURAPP/configurationvariables/0, AppHarbor replies with:
{
"key": null,
"value": null
}

Also, when I went to run the tests, it looks like there's a lot of broken tests. I didn't write a test for my change.

[ntziolis]

Can you elaborate on the Url returned with the value from AppHarbor is null? Do you mean the URL property of the json result is null?
Could you post an obfuscated raw result of the GetConfigurationValues call?

[gmcelhanon]

Here's the code snippet for the AppHarbor request...

RestRequest request = new RestRequest();
request.Resource = "applications/{applicationID}/configurationvariables";
request.AddParameter("applicationID", (object)applicationID, ParameterType.UrlSegment);

string content = client.Execute(request).Content;

"content" return from AppHarbor looks like this (note the null Url):

[
  {
    "key": "CLOUDAMQP_URL",
    "value": "amqp://[email protected]/000...70af5_apphb.com",
    "url": null
  }
]

This configuration value was supplied by the RabbitMQ host in the add-ons section. Since they control the value, AppHarbor says that I should programmatically retrieve it since it could change. But there is no "id", nor "url" for the value. I have to retrieve all of the values and then look for it by key.

[ntziolis]

Thx for the fast feedback. The url values is set by the AppHarbor backend and is not supplied by RabbitMQ. I think there is an error with the AppHarbor backend and I'm gonna contact them. I'll report back here on this issue.

[tt]

I think one thing we should do on our end is to use the key as the ID. This makes a lot more sense to me. Unfortunately this will break with current consumers of this library as it tries parsing the ID to a long. Is there any reason for this behavior? I think it would be generally preferable if all IDs are treated as string so the client doesn't make assumptions about our implementation.

Incidentally, using the key as ID will solve this issue as we'll be able to retrieve the value. The reason we can't do this currently is because add-on related variables isn't persisted the same way as user-added ones. Naturally, you'll not be able to delete add-on related variables, but trying should just yield a not allowed response.

[ntziolis]

In general I have nothing against using string for IDs. If compatibility is an issue only config variable ID's could be string in order to break as little as possible. I will look into this and get back here in this thread. Might take a couple of days though.