On 06/12/2012 02:38 PM, Simo Sorce wrote:
On Tue, 2012-06-12 at 13:12 +0200, Petr Viktorin wrote:
This will make older clients usable if new output items get added to
commands.
Since there might be important information in the extra output, it's not
ignored as the ticket asks. Instead it's printed, but not formatted
nicely as the client doesn't have enough info for that.
https://fedorahosted.org/freeipa/ticket/1721
Patch is missing.
Simo.
My apologies
--
Petr³
From e039780d0038b57b621620eb429061a3fdfa311c Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Tue, 5 Jun 2012 10:26:35 -0400
Subject: [PATCH] Don't crash when server returns extra output
When input new optional parameter is added to server API, its still
usable with old clients with old API, it just cannot use the new parameter.
However, when a new Output parameter is added, older clients immediately
failed with a validation error.
This patch relaxes the validation so that extra Output items are allowed
in the client.
The extra items are displayed at the end of a command's output (crudely
formatted and with raw names, since the older client doesn't have any
formatting info).
This should ensure that important messages are not lost.
On the server, the validation still does not allow extra Output items.
Also, fix an error where if the expected and actual number of Output
items was the same, validation succeeded even if the keys were different.
https://fedorahosted.org/freeipa/ticket/1721
---
ipalib/frontend.py | 32 +++++++++++++------
tests/test_ipalib/test_frontend.py | 59 +++++++++++++++++++++++++++---------
2 files changed, 67 insertions(+), 24 deletions(-)
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 10087ba24bc310a036a22d4c52740825de3184ce..e05638b2f42025338aa5c6f35e7166462ca8f196 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -904,18 +904,22 @@ def validate_output(self, output):
raise TypeError('%s: need a %r; got a %r: %r' % (
nice, dict, type(output), output)
)
- if len(output) < len(self.output):
- missing = sorted(set(self.output).difference(output))
+ missing = set(self.output).difference(output)
+ if missing:
raise ValueError('%s: missing keys %r in %r' % (
- nice, missing, output)
- )
- if len(output) > len(self.output):
- extra = sorted(set(output).difference(self.output))
- raise ValueError('%s: unexpected keys %r in %r' % (
- nice, extra, output)
+ nice, sorted(missing), output)
)
+ if self.api.env.in_server:
+ extra = set(output).difference(self.output)
+ if extra:
+ raise ValueError('%s: unexpected keys %r in %r' % (
+ nice, sorted(extra), output)
+ )
for o in self.output():
- value = output[o.name]
+ try:
+ value = output[o.name]
+ except KeyError:
+ continue
if not (o.type is None or isinstance(value, o.type)):
raise TypeError('%s:\n output[%r]: need %r; got %r: %r' % (
nice, o.name, o.type, type(value), value)
@@ -999,6 +1003,16 @@ def output_for_cli(self, textui, output, *args, **options):
elif isinstance(result, int):
textui.print_count(result, '%s %%d' % unicode(self.output[o].doc))
+ # If there is extra output from the server, it probably means that
+ # the server is newer and an output item was added.
+ # Since the item might be important, we show it even though we can't
+ # format it correctly
+ extra = set(output).difference(self.output)
+ if extra:
+ textui.print_dashed(unicode(_('Unexpected output from server:')))
+ for item in sorted(extra):
+ textui.print_line('%s: %s' % (item, output[item]))
+
return rv
# list of attributes we want exported to JSON
diff --git a/tests/test_ipalib/test_frontend.py b/tests/test_ipalib/test_frontend.py
index b717a43ad4c68940582f901308f4dd4da77834ee..dcdea694a34adbf0e351cbfa924b2f2b0666b06c 100644
--- a/tests/test_ipalib/test_frontend.py
+++ b/tests/test_ipalib/test_frontend.py
@@ -610,69 +610,98 @@ def forward(self, *args, **kw):
assert o.run.im_func is self.cls.run.im_func
assert ('forward', args, kw) == o.run(*args, **kw)
- def test_validate_output(self):
+ def test_validate_output_basic(self):
"""
Test the `ipalib.frontend.Command.validate_output` method.
"""
- class Example(self.cls):
+ class example(self.cls):
has_output = ('foo', 'bar', 'baz')
- inst = Example()
- inst.finalize()
+ (api, home) = create_test_api(in_server=True)
+ api.register(example)
+
+ api.finalize()
+ inst = api.Command.example
# Test with wrong type:
wrong = ('foo', 'bar', 'baz')
e = raises(TypeError, inst.validate_output, wrong)
assert str(e) == '%s.validate_output(): need a %r; got a %r: %r' % (
- 'Example', dict, tuple, wrong
+ 'example', dict, tuple, wrong
)
# Test with a missing keys:
wrong = dict(bar='hello')
e = raises(ValueError, inst.validate_output, wrong)
assert str(e) == '%s.validate_output(): missing keys %r in %r' % (
- 'Example', ['baz', 'foo'], wrong
+ 'example', ['baz', 'foo'], wrong
)
- # Test with extra keys:
+ # Test with extra keys (api.env.in_server is set, so this raises):
wrong = dict(foo=1, bar=2, baz=3, fee=4, azz=5)
e = raises(ValueError, inst.validate_output, wrong)
assert str(e) == '%s.validate_output(): unexpected keys %r in %r' % (
- 'Example', ['azz', 'fee'], wrong
+ 'example', ['azz', 'fee'], wrong
)
+ # Test with different keys:
+ wrong = dict(baz=1, xyzzy=2, quux=3)
+ e = raises(ValueError, inst.validate_output, wrong)
+ assert str(e) == '%s.validate_output(): missing keys %r in %r' % (
+ 'example', ['bar', 'foo'], wrong
+ ), str(e)
+
+ def test_validate_output_per_item(self):
+ """
+ Test the `ipalib.frontend.Command.validate_output` method.
+ """
# Test with per item type validation:
- class Complex(self.cls):
+ class complex_command(self.cls):
has_output = (
output.Output('foo', int),
output.Output('bar', list),
)
- inst = Complex()
- inst.finalize()
+ (api, home) = create_test_api(in_server=False)
+ api.register(complex_command)
+
+ api.finalize()
+ inst = api.Command.complex_command
wrong = dict(foo=17.9, bar=[18])
e = raises(TypeError, inst.validate_output, wrong)
assert str(e) == '%s:\n output[%r]: need %r; got %r: %r' % (
- 'Complex.validate_output()', 'foo', int, float, 17.9
+ 'complex_command.validate_output()', 'foo', int, float, 17.9
)
wrong = dict(foo=18, bar=17)
e = raises(TypeError, inst.validate_output, wrong)
assert str(e) == '%s:\n output[%r]: need %r; got %r: %r' % (
- 'Complex.validate_output()', 'bar', list, int, 17
+ 'complex_command.validate_output()', 'bar', list, int, 17
)
+ # Test with extra keys (api.env.in_server not set, no error raised)
+ wrong = dict(foo=1, bar=[2], baz=3, fee=4, azz=5)
+ inst.validate_output(wrong)
+
+ def test_validate_output_nested(self):
+ """
+ Test the `ipalib.frontend.Command.validate_output` method.
+ """
class Subclass(output.ListOfEntries):
pass
# Test nested validation:
class nested(self.cls):
has_output = (
output.Output('hello', int),
Subclass('world'),
)
- inst = nested()
- inst.finalize()
+ (api, home) = create_test_api(in_server=False)
+ api.register(nested)
+
+ api.finalize()
+ inst = api.Command.nested
+
okay = dict(foo='bar')
nope = ('aye', 'bee')
--
1.7.10.2
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
