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

Bug 1381509 - fix public api swagger errors #360

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions auslib/test/web/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import json

import auslib.web.public.client as client_api
from auslib.web.public.client import update_query_version

from auslib.blobs.base import createBlob
from auslib.global_state import dbo
Expand Down Expand Up @@ -907,6 +908,54 @@ def testNonSubstitutedUrlVariablesReturn404(self):
self.assertEqual(ret.status_code, 404)
self.assertFalse(mock_cr_view.called)

def testUpdateQueryVersion(self):
version_key = 'queryVersion'
url_params = {}
update_query_version('/update/1/a/b/c.xml', url_params)
self.assertIn(version_key, url_params)
self.assertIn('osVersion', url_params)
self.assertEqual(url_params[version_key], 1)

url_params = {}
update_query_version('/update/2/a/b/c.xml', url_params)
self.assertIn(version_key, url_params)
self.assertEqual(url_params[version_key], 2)

url_params = {}
update_query_version('/update/3/a/b/c.xml', url_params)
self.assertIn(version_key, url_params)
self.assertEqual(url_params[version_key], 3)

url_params = {}
update_query_version('/update/4/a/b/c.xml', url_params)
self.assertIn(version_key, url_params)
self.assertEqual(url_params[version_key], 4)

url_params = {}
update_query_version('/update/5/a/b/c.xml', url_params)
self.assertIn(version_key, url_params)
self.assertEqual(url_params[version_key], 5)

url_params = {}
update_query_version('/update/6/a/b/c.xml', url_params)
self.assertIn(version_key, url_params)
self.assertEqual(url_params[version_key], 6)

url_params = {}
update_query_version('/update/7/a/b/c.xml', url_params)
self.assertIn(version_key, url_params)
self.assertEqual(url_params[version_key], 0)

url_params = {}
update_query_version('/update/x/a/b/c.xml', url_params)
self.assertIn(version_key, url_params)
self.assertEqual(url_params[version_key], 0)

url_params = {}
update_query_version('/update/x1/a/b/c.xml', url_params)
self.assertIn(version_key, url_params)
self.assertEqual(url_params[version_key], 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Good job with this test coverage.



class ClientTestWithErrorHandlers(ClientTestCommon):
"""Most of the tests are run without the error handler because it gives more
Expand Down
5 changes: 0 additions & 5 deletions auslib/web/api_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ def validate_path_parameter(self, param, request):
# Keep the base class behavior.
val = request.path_params.get(param['name'].replace('-', '_'))

# Supporting the flask add_url_rule "defaults" parameter to the swagger path parameter.
if val is None and 'default' in param:
val = param['default']
request.path_params[param['name']] = val

# We don't expect any unicode in the update URL, nor do we know which
# encoding it would use if we get it, so we simply ignore any that is
# given. All known cases of this are misconfiguration on the client end.
Expand Down
2 changes: 1 addition & 1 deletion auslib/web/common/releases_spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ definitions:
product:
type: string
read_only:
type: ['boolean', 'null']
type: boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite understand why adding "null" to most of the properties is necessary. As far as I know, the API always returns them, so why is null valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I comment about this bellow.

rule_ids:
type: array
items:
Expand Down
36 changes: 18 additions & 18 deletions auslib/web/common/rules_spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,21 +73,21 @@ definitions:
format: int32
description: The id of the rule. This id (or the rule's alias, if set) is necessary to make changes to the rule through the REST API.
alias:
type: ['string', 'null']
type: string
description: The alias of the rule. This value must be unique across the entire set of rules.
priority:
type: ['integer', 'null']
type: integer
format: int32
description: >
The priority of the rule, relative to other rules. The higher the value, the higher the priority of the rule.
mapping:
type: ['string', 'null']
type: string
description: The Release to construct an update out of if the user is on the right side of a background rate dice roll, or if the background rate is 100. This is a foreign key to the "name" column of the Releases table.
fallbackMapping:
type: ['string', 'null']
type: string
description: The Release to construct an update out of when the user is on the wrong side of a background rate dice roll. This is a foreign key to the "name" column of the Releases table.
backgroundRate:
type: ['integer', 'null']
type: integer
format: int32
description: >
The percentage of background update requests to serve the "mapping" to.
Expand All @@ -96,46 +96,46 @@ definitions:
type: string
description: The update_type to use in the XML response. "minor" is almost always the correct value these days.
product:
type: ['string', 'null']
type: string
description: The product name that the client must send in order for the rule to match. Only full string matches are supported.
version:
type: ['string', 'null']
type: string
description: >
The version requirements that the client must meet in order for the rule to match. Exact matches, comma separated lists (eg: "36.0,36.0.1") and comparisons (eg: "<45.0") are allowed.
channel:
type: ['string', 'null']
type: string
description: The update channel that the client must send in order for the rule to match. "*" can be used to glob at the end of the channel name.
buildTarget:
type: ['string', 'null']
type: string
description: The “build target” that the client must send in order for the rule to match. This is usually related to the target platform the app was built for. Only full string matches are supported.
buildID:
type: ['string', 'null']
type: string
description: The build ID requirements that the client must meet in order for the rule to match. Comparisons such as "<20170504030201" may be used.
locale:
type: ['string', 'null']
type: string
description: The locale that the client must send in order for the rule to match. A comma separated list may be used to list multiple locales.
osVersion:
type: ['string', 'null']
type: string
description: >
The OS Version requirements that the client must meet in order for the rule to match. && may be used to match multiple substrings at once (eg: "Windows && (websense-"). Commas may be used to match any of a list of substrings.
instructionSet:
type: ['string', 'null']
type: string
description: >
The most modern CPU instruction set that the client must support in order for the rule to match. A comma separated list may be used to list multiple instruction sets.
memory:
type: ['string', 'null']
type: string
description: The amount of RAM, in megabytes, that the client must have for the rule to match. Comparisons such as "<8000" may be used.
distribution:
type: ['string', 'null']
type: string
description: The partner distribution name that the application must send in order for the rule to match.
distVersion:
type: ['string', 'null']
type: string
description: The version of the partner distribution that the application must send in order for the rule to match.
headerArchitecture:
type: ['string', 'null']
type: string
description: The client architecture (as guessed based on the User Agent) that the client must provide in order for the rule to match. This field is deprecated in favour of build target.
comment:
type: ['string', 'null']
type: string
description: A string describing the purpose of the rule. Not always necessary for obvious rules.

RuleHistory:
Expand Down
52 changes: 0 additions & 52 deletions auslib/web/public/api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,6 @@ paths:
- $ref: '#/parameters/buildTarget'
- $ref: '#/parameters/locale'
- $ref: '#/parameters/channel'
- name: queryVersion
in: path
description: queryVersion
required: true
type: integer
default: 1
# Underlying code depends on osVersion being set. Since this route only
# exists to support ancient queries, and all newer versions have osVersion
# in them it's easier to set this here than make the all of the underlying
# code support queries without it.
- name: osVersion
in: path
description: The OS Version of the application requesting an update. This field is primarily used to point desupported operating systems to their last supported build.
required: true
type: string
default: ""
- $ref: '#/parameters/avast'
- $ref: '#/parameters/force'
responses:
Expand All @@ -65,12 +49,6 @@ paths:
- $ref: '#/parameters/locale'
- $ref: '#/parameters/channel'
- $ref: '#/parameters/osVersion'
- name: queryVersion
in: path
description: queryVersion
required: true
type: integer
default: 2
- $ref: '#/parameters/avast'
- $ref: '#/parameters/force'
responses:
Expand All @@ -93,12 +71,6 @@ paths:
- $ref: '#/parameters/osVersion'
- $ref: '#/parameters/distribution'
- $ref: '#/parameters/distVersion'
- name: queryVersion
in: path
description: queryVersion
required: true
type: integer
default: 3
- $ref: '#/parameters/avast'
- $ref: '#/parameters/force'
responses:
Expand Down Expand Up @@ -126,12 +98,6 @@ paths:
description: Platform Version.
required: true
type: string
- name: queryVersion
in: path
description: queryVersion
required: true
type: integer
default: 4
- $ref: '#/parameters/avast'
- $ref: '#/parameters/force'
responses:
Expand Down Expand Up @@ -159,12 +125,6 @@ paths:
description: IMEI
required: true
type: string
- name: queryVersion
in: path
description: queryVersion
required: true
type: integer
default: 5
- $ref: '#/parameters/avast'
- $ref: '#/parameters/force'
responses:
Expand Down Expand Up @@ -193,12 +153,6 @@ paths:
type: string
- $ref: '#/parameters/distribution'
- $ref: '#/parameters/distVersion'
- name: queryVersion
in: path
description: queryVersion
required: true
type: integer
default: 6
- $ref: '#/parameters/avast'
- $ref: '#/parameters/force'
responses:
Expand All @@ -223,12 +177,6 @@ paths:
- $ref: '#/parameters/osVersion'
- $ref: '#/parameters/distribution'
- $ref: '#/parameters/distVersion'
- name: queryVersion
in: path
description: queryVersion
required: true
type: integer
default: 3
- $ref: '#/parameters/avast'
- $ref: '#/parameters/force'
responses:
Expand Down
1 change: 0 additions & 1 deletion auslib/web/public/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
.add_spec(path.join(web_dir, 'common/releases_spec.yml'))\
.add_spec(path.join(web_dir, 'common/rules_spec.yml'))
connexion_app.add_api(spec,
validate_responses=True,
strict_validation=True)


Expand Down
24 changes: 24 additions & 0 deletions auslib/web/public/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,31 @@ def getQueryFromURL(url):
return query


def extract_query_version(request_url):
version = 0
pattern = '^.*/update/([1-6])/.*\.xml$'
match = re.match(pattern, request_url)
if match:
version = int(match.group(1))
return version


def update_query_version(request_url, url_params):
version = extract_query_version(request_url)
defaults = {'queryVersion': version}

# Underlying code depends on osVersion being set. Since this route only
# exists to support ancient queries, and all newer versions have osVersion
# in them it's easier to set this here than make the all of the underlying
# code support queries without it.
if version == 1:
defaults['osVersion'] = ""

url_params.update(defaults)


def get_update_blob(**url):
update_query_version(request.url, url)
query = getQueryFromURL(url)
LOG.debug("Got query: %s", query)
release, update_type = AUS.evaluateRules(query)
Expand Down