From ff0d18e6077bd235ef20d9f4599479a071dedd77 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Sat, 15 Jan 2011 15:25:24 +0100
Subject: [PATCH] Check consistency of the class names and OP_ID
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
As the class names should be now consistent with the OP_IDs, we add a
check for wrongly-defined OP_IDs.
However, the future removal of the hand-coded OP_IDs will render this
obsolete, so this check is introduced just to make sure that the
previous renaming patches did the right job, and it will then be
removed.
The consistency checks require renaming the test opcodes, which were
using arbitrary names, depending on test author. They are now all
standardized on OpTest (local scope).
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: RenΓ© Nussbaumer <rn@google.com>
---
lib/opcodes.py | 46 +++++++++++++++++++++++----
test/ganeti.cmdlib_unittest.py | 9 +++---
test/ganeti.opcodes_unittest.py | 41 +++++++++++-------------
test/ganeti.rapi.baserlib_unittest.py | 31 +++++++++---------
4 files changed, 78 insertions(+), 49 deletions(-)
diff --git a/lib/opcodes.py b/lib/opcodes.py
index 1d3179c6a..ff7ec1d3b 100644
--- a/lib/opcodes.py
+++ b/lib/opcodes.py
@@ -34,6 +34,7 @@ opcodes.
# pylint: disable-msg=R0903
import logging
+import re
from ganeti import constants
from ganeti import errors
@@ -77,6 +78,30 @@ _PTagKind = ("kind", ht.NoDefault, ht.TElemOf(constants.VALID_TAG_TYPES))
#: List of tag strings
_PTags = ("tags", ht.NoDefault, ht.TListOf(ht.TNonEmptyString))
+#: OP_ID conversion regular expression
+_OPID_RE = re.compile("([a-z])([A-Z])")
+
+
+def _NameToId(name):
+ """Convert an opcode class name to an OP_ID.
+
+ @type name: string
+ @param name: the class name, as OpXxxYyy
+ @rtype: string
+ @return: the name in the OP_XXXX_YYYY format
+
+ """
+ if not name.startswith("Op"):
+ return None
+ # Note: (?<=[a-z])(?=[A-Z]) would be ideal, since it wouldn't
+ # consume any input, and hence we would just have all the elements
+ # in the list, one by one; but it seems that split doesn't work on
+ # non-consuming input, hence we have to process the input string a
+ # bit
+ name = _OPID_RE.sub(r"\1,\2", name)
+ elems = name.split(",")
+ return "_".join(n.upper() for n in elems)
+
def RequireFileStorage():
"""Checks that file storage is enabled.
@@ -138,7 +163,14 @@ class _AutoOpParamSlots(type):
"""
assert "__slots__" not in attrs, \
"Class '%s' defines __slots__ when it should use OP_PARAMS" % name
- assert "OP_ID" in attrs, "Class '%s' is missing OP_ID attribute" % name
+
+ op_id = _NameToId(name)
+ if "OP_ID" in attrs:
+ assert attrs["OP_ID"] == op_id, ("Class '%s' defining wrong OP_ID"
+ " attribute %s, should be %s" %
+ (name, attrs["OP_ID"], op_id))
+
+ attrs["OP_ID"] = op_id
# Always set OP_PARAMS to avoid duplicates in BaseOpCode.GetAllParams
params = attrs.setdefault("OP_PARAMS", [])
@@ -296,7 +328,7 @@ class OpCode(BaseOpCode):
@ivar priority: Opcode priority for queue
"""
- OP_ID = "OP_ABSTRACT"
+ OP_ID = "OP_CODE"
WITH_LU = True
OP_PARAMS = [
("dry_run", None, ht.TMaybeBool),
@@ -352,12 +384,14 @@ class OpCode(BaseOpCode):
def Summary(self):
"""Generates a summary description of this opcode.
- The summary is the value of the OP_ID attribute (without the "OP_" prefix),
- plus the value of the OP_DSC_FIELD attribute, if one was defined; this field
- should allow to easily identify the operation (for an instance creation job,
- e.g., it would be the instance name).
+ The summary is the value of the OP_ID attribute (without the "OP_"
+ prefix), plus the value of the OP_DSC_FIELD attribute, if one was
+ defined; this field should allow to easily identify the operation
+ (for an instance creation job, e.g., it would be the instance
+ name).
"""
+ assert self.OP_ID is not None and len(self.OP_ID) > 3
# all OP_ID start with OP_, we remove that
txt = self.OP_ID[3:]
field_name = getattr(self, "OP_DSC_FIELD", None)
diff --git a/test/ganeti.cmdlib_unittest.py b/test/ganeti.cmdlib_unittest.py
index 234178958..46e3e8b87 100755
--- a/test/ganeti.cmdlib_unittest.py
+++ b/test/ganeti.cmdlib_unittest.py
@@ -1,7 +1,7 @@
#!/usr/bin/python
#
-# Copyright (C) 2008 Google Inc.
+# Copyright (C) 2008, 2011 Google Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -85,9 +85,8 @@ class TestIAllocatorChecks(testutils.GanetiTestCase):
self.cfg = mocks.FakeConfig()
self.op = opcode
- class TestOpcode(opcodes.OpCode):
- OP_ID = "OP_TEST"
- OP_PARAMS = [
+ class OpTest(opcodes.OpCode):
+ OP_PARAMS = [
("iallocator", None, ht.NoType),
("node", None, ht.NoType),
]
@@ -95,7 +94,7 @@ class TestIAllocatorChecks(testutils.GanetiTestCase):
default_iallocator = mocks.FakeConfig().GetDefaultIAllocator()
other_iallocator = default_iallocator + "_not"
- op = TestOpcode()
+ op = OpTest()
lu = TestLU(op)
c_i = lambda: cmdlib._CheckIAllocatorOrNode(lu, "iallocator", "node")
diff --git a/test/ganeti.opcodes_unittest.py b/test/ganeti.opcodes_unittest.py
index bb77ca211..6c2fdb04b 100755
--- a/test/ganeti.opcodes_unittest.py
+++ b/test/ganeti.opcodes_unittest.py
@@ -45,6 +45,7 @@ class TestOpcodes(unittest.TestCase):
self.assert_(cls.OP_ID.startswith("OP_"))
self.assert_(len(cls.OP_ID) > 3)
self.assertEqual(cls.OP_ID, cls.OP_ID.upper())
+ self.assertEqual(cls.OP_ID, opcodes._NameToId(cls.__name__))
self.assertRaises(TypeError, cls, unsupported_parameter="some value")
@@ -88,32 +89,30 @@ class TestOpcodes(unittest.TestCase):
self.assertEqual("OP_%s" % summary, op.OP_ID)
def testSummary(self):
- class _TestOp(opcodes.OpCode):
- OP_ID = "OP_TEST"
+ class OpTest(opcodes.OpCode):
OP_DSC_FIELD = "data"
OP_PARAMS = [
("data", ht.NoDefault, ht.TString),
]
- self.assertEqual(_TestOp(data="").Summary(), "TEST()")
- self.assertEqual(_TestOp(data="Hello World").Summary(),
+ self.assertEqual(OpTest(data="").Summary(), "TEST()")
+ self.assertEqual(OpTest(data="Hello World").Summary(),
"TEST(Hello World)")
- self.assertEqual(_TestOp(data="node1.example.com").Summary(),
+ self.assertEqual(OpTest(data="node1.example.com").Summary(),
"TEST(node1.example.com)")
def testListSummary(self):
- class _TestOp(opcodes.OpCode):
- OP_ID = "OP_TEST"
+ class OpTest(opcodes.OpCode):
OP_DSC_FIELD = "data"
OP_PARAMS = [
("data", ht.NoDefault, ht.TList),
]
- self.assertEqual(_TestOp(data=["a", "b", "c"]).Summary(),
+ self.assertEqual(OpTest(data=["a", "b", "c"]).Summary(),
"TEST(a,b,c)")
- self.assertEqual(_TestOp(data=["a", None, "c"]).Summary(),
+ self.assertEqual(OpTest(data=["a", None, "c"]).Summary(),
"TEST(a,None,c)")
- self.assertEqual(_TestOp(data=[1, 2, 3, 4]).Summary(), "TEST(1,2,3,4)")
+ self.assertEqual(OpTest(data=[1, 2, 3, 4]).Summary(), "TEST(1,2,3,4)")
def testOpId(self):
self.assertFalse(utils.FindDuplicates(cls.OP_ID
@@ -162,8 +161,7 @@ class TestOpcodes(unittest.TestCase):
msg="Default value returned by function is callable")
def testValidateNoModification(self):
- class _TestOp(opcodes.OpCode):
- OP_ID = "OP_TEST"
+ class OpTest(opcodes.OpCode):
OP_PARAMS = [
("nodef", ht.NoDefault, ht.TMaybeString),
("wdef", "default", ht.TMaybeString),
@@ -172,7 +170,7 @@ class TestOpcodes(unittest.TestCase):
]
# Missing required parameter "nodef"
- op = _TestOp()
+ op = OpTest()
before = op.__getstate__()
self.assertRaises(errors.OpPrereqError, op.Validate, False)
self.assertFalse(hasattr(op, "nodef"))
@@ -182,7 +180,7 @@ class TestOpcodes(unittest.TestCase):
self.assertEqual(op.__getstate__(), before, msg="Opcode was modified")
# Required parameter "nodef" is provided
- op = _TestOp(nodef="foo")
+ op = OpTest(nodef="foo")
before = op.__getstate__()
op.Validate(False)
self.assertEqual(op.__getstate__(), before, msg="Opcode was modified")
@@ -192,7 +190,7 @@ class TestOpcodes(unittest.TestCase):
self.assertFalse(hasattr(op, "notype"))
# Missing required parameter "nodef"
- op = _TestOp(wdef="hello", number=999)
+ op = OpTest(wdef="hello", number=999)
before = op.__getstate__()
self.assertRaises(errors.OpPrereqError, op.Validate, False)
self.assertFalse(hasattr(op, "nodef"))
@@ -200,7 +198,7 @@ class TestOpcodes(unittest.TestCase):
self.assertEqual(op.__getstate__(), before, msg="Opcode was modified")
# Wrong type for "nodef"
- op = _TestOp(nodef=987)
+ op = OpTest(nodef=987)
before = op.__getstate__()
self.assertRaises(errors.OpPrereqError, op.Validate, False)
self.assertEqual(op.nodef, 987)
@@ -208,14 +206,14 @@ class TestOpcodes(unittest.TestCase):
self.assertEqual(op.__getstate__(), before, msg="Opcode was modified")
# Testing different types for "notype"
- op = _TestOp(nodef="foo", notype=[1, 2, 3])
+ op = OpTest(nodef="foo", notype=[1, 2, 3])
before = op.__getstate__()
op.Validate(False)
self.assertEqual(op.nodef, "foo")
self.assertEqual(op.notype, [1, 2, 3])
self.assertEqual(op.__getstate__(), before, msg="Opcode was modified")
- op = _TestOp(nodef="foo", notype="Hello World")
+ op = OpTest(nodef="foo", notype="Hello World")
before = op.__getstate__()
op.Validate(False)
self.assertEqual(op.nodef, "foo")
@@ -223,8 +221,7 @@ class TestOpcodes(unittest.TestCase):
self.assertEqual(op.__getstate__(), before, msg="Opcode was modified")
def testValidateSetDefaults(self):
- class _TestOp(opcodes.OpCode):
- OP_ID = "OP_TEST"
+ class OpTest(opcodes.OpCode):
OP_PARAMS = [
# Static default value
("value1", "default", ht.TMaybeString),
@@ -233,7 +230,7 @@ class TestOpcodes(unittest.TestCase):
("value2", lambda: "result", ht.TMaybeString),
]
- op = _TestOp()
+ op = OpTest()
before = op.__getstate__()
op.Validate(True)
self.assertNotEqual(op.__getstate__(), before,
@@ -244,7 +241,7 @@ class TestOpcodes(unittest.TestCase):
self.assert_(op.debug_level is None)
self.assertEqual(op.priority, constants.OP_PRIO_DEFAULT)
- op = _TestOp(value1="hello", value2="world", debug_level=123)
+ op = OpTest(value1="hello", value2="world", debug_level=123)
before = op.__getstate__()
op.Validate(True)
self.assertNotEqual(op.__getstate__(), before,
diff --git a/test/ganeti.rapi.baserlib_unittest.py b/test/ganeti.rapi.baserlib_unittest.py
index 570636fa7..807c34d27 100755
--- a/test/ganeti.rapi.baserlib_unittest.py
+++ b/test/ganeti.rapi.baserlib_unittest.py
@@ -33,53 +33,52 @@ import testutils
class TestFillOpcode(unittest.TestCase):
- class _TestOp(opcodes.OpCode):
- OP_ID = "RAPI_TEST_OP"
+ class OpTest(opcodes.OpCode):
OP_PARAMS = [
("test", None, ht.TMaybeString),
]
def test(self):
for static in [None, {}]:
- op = baserlib.FillOpcode(self._TestOp, {}, static)
- self.assertTrue(isinstance(op, self._TestOp))
+ op = baserlib.FillOpcode(self.OpTest, {}, static)
+ self.assertTrue(isinstance(op, self.OpTest))
self.assertFalse(hasattr(op, "test"))
def testStatic(self):
- op = baserlib.FillOpcode(self._TestOp, {}, {"test": "abc"})
- self.assertTrue(isinstance(op, self._TestOp))
+ op = baserlib.FillOpcode(self.OpTest, {}, {"test": "abc"})
+ self.assertTrue(isinstance(op, self.OpTest))
self.assertEqual(op.test, "abc")
# Overwrite static parameter
self.assertRaises(http.HttpBadRequest, baserlib.FillOpcode,
- self._TestOp, {"test": 123}, {"test": "abc"})
+ self.OpTest, {"test": 123}, {"test": "abc"})
def testType(self):
self.assertRaises(http.HttpBadRequest, baserlib.FillOpcode,
- self._TestOp, {"test": [1, 2, 3]}, {})
+ self.OpTest, {"test": [1, 2, 3]}, {})
def testStaticType(self):
self.assertRaises(http.HttpBadRequest, baserlib.FillOpcode,
- self._TestOp, {}, {"test": [1, 2, 3]})
+ self.OpTest, {}, {"test": [1, 2, 3]})
def testUnicode(self):
- op = baserlib.FillOpcode(self._TestOp, {u"test": "abc"}, {})
- self.assertTrue(isinstance(op, self._TestOp))
+ op = baserlib.FillOpcode(self.OpTest, {u"test": "abc"}, {})
+ self.assertTrue(isinstance(op, self.OpTest))
self.assertEqual(op.test, "abc")
- op = baserlib.FillOpcode(self._TestOp, {}, {u"test": "abc"})
- self.assertTrue(isinstance(op, self._TestOp))
+ op = baserlib.FillOpcode(self.OpTest, {}, {u"test": "abc"})
+ self.assertTrue(isinstance(op, self.OpTest))
self.assertEqual(op.test, "abc")
def testUnknownParameter(self):
self.assertRaises(http.HttpBadRequest, baserlib.FillOpcode,
- self._TestOp, {"othervalue": 123}, None)
+ self.OpTest, {"othervalue": 123}, None)
def testInvalidBody(self):
self.assertRaises(http.HttpBadRequest, baserlib.FillOpcode,
- self._TestOp, "", None)
+ self.OpTest, "", None)
self.assertRaises(http.HttpBadRequest, baserlib.FillOpcode,
- self._TestOp, range(10), None)
+ self.OpTest, range(10), None)
if __name__ == "__main__":
--
GitLab