Skip to content

Commit

Permalink
Bug with segfault when getting data in form of map<...> asking variat…
Browse files Browse the repository at this point in the history
…ion where no such data exist - fixed.

Regression unit test for the bug
Java Kotlin runtime libraries update
SQL Schema update - default parent variation ID now 1 that corresponds to default Variation
-p or --parent flag for mkvar to specify the parent Variation
'info' command now shows variation parent 


git-svn-id: https://phys12svn.jlab.org/repos/trunk/ccdb@2017 c5ed4466-e916-0410-8347-b3263e9c103d
  • Loading branch information
DmitryRomanovTest committed Jun 25, 2014
1 parent fad5eca commit b1506ef
Show file tree
Hide file tree
Showing 15 changed files with 134 additions and 145 deletions.
154 changes: 60 additions & 94 deletions java/.idea/workspace.xml

Large diffs are not rendered by default.

Binary file modified java/lib/kotlin-runtime-sources.jar
Binary file not shown.
Binary file modified java/lib/kotlin-runtime.jar
Binary file not shown.
1 change: 1 addition & 0 deletions python/.idea/dictionaries/Dmitry.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion python/ccdb/cmd/console_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ def print_info(self):
def print_interactive_intro(self):
print """
+--------------------------+
CCDB shell v.1.00
CCDB shell v.1.02
HallD JLab
+--------------------------+
"""
Expand Down
1 change: 1 addition & 0 deletions python/ccdb/cmd/utils/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ def print_variation(self, variation):
print " Name : " + self.theme.Success + variation.name
print " Created : " + variation.created.strftime("%Y-%m-%d %H-%M-%S")
print " DB Id : " + repr(int(variation.id))
print " Parent : " + (variation.parent.name if variation.parent else "--")
print " Comment: "
print variation.comment
print
Expand Down
62 changes: 32 additions & 30 deletions python/ccdb/cmd/utils/mkvar.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from ccdb.cmd import ConsoleUtilBase
from ccdb.cmd import ConsoleUtilBase, UtilityArgumentParser
from ccdb.path_utils import validate_name
import logging

log = logging.getLogger("ccdb.cmd.utils.mkvar")


#ccdbcmd module interface
def create_util_instance():
log.debug(" registering MakeVariation")
Expand All @@ -16,7 +17,7 @@ def create_util_instance():
#*********************************************************************
class MakeVariation(ConsoleUtilBase):
""" Create variation """

# ccdb utility class descr part
#------------------------------
command = "mkvar"
Expand All @@ -28,54 +29,55 @@ class MakeVariation(ConsoleUtilBase):
def process(self, args):
log.debug("MakeVariation module gained control")
log.debug("Arguments: \n " + " ".join(args))

if not len(args): return

comment = ""
raw_str = args[0] #hello dynamic language!
if not len(args):
return

#try to extract comment from /path/#comment
if "#" in raw_str:
var_name = raw_str[:raw_str.index("#")]
comment = raw_str[raw_str.index("#") + 1:]
else:
var_name = raw_str
#find #comment
comment = ""
for i in range(len(args)):
arg = args[i]
if arg.startswith("#"):
comment = (" ".join(args[i:]))[1:] # [1:] to remove #
args = args[:i]
break

#utility argument parser is argparse which raises errors instead of exiting app
parser = UtilityArgumentParser()
parser.add_argument("name", default="")
parser.add_argument("-p", "--parent", default="")
result = parser.parse_args(args)

parser.add_argument("--verbose", help="increase output verbosity",
action="store_true")

#in case there is a space between comments and name

if len(args)>=2 and str(args[1]).startswith("#"):
comment = comment + args[1][1:]
if len(args)>=3:
comment = comment + " " + " ".join(args[2:])

if not validate_name(var_name):
log.warning("Invalid variation name. Only [a-z A-Z 0-9 _] symbols are allowed for variation name")
return 1
if not validate_name(result.name):
raise ValueError("Invalid variation name. Only [a-z A-Z 0-9 _] symbols are allowed for variation name")

#try to create directory
log.debug(" creating variation. Name: {0}, comment: {1}".format(var_name, comment))

try:
self.context.provider.create_variation(var_name, comment)
except Exception as ex:
log.warning("Failed to create directory. Exception message: {0}".format(ex))
log.debug(" creating variation. Name: {0}, comment: {1}".format(result.name, comment))

log.info("Variation " + var_name + self.theme.Success + " created" + self.theme.Reset)
self.context.provider.create_variation(result.name, comment, result.parent)

log.info("Variation " + result.name + self.theme.Success + " created" + self.theme.Reset)

#----------------------------------------------
# print_help - prints help
#----------------------------------------------
def print_help(self):
"prints help to user"
"""prints help to user"""

print """
MakeVariation or mkvar - create variation with specified name
usage:
mkvar <name> #<comments>
mkvar <name> -p <parent> #<comments>
name - is a variation name. [a-z A-Z 0-9 _] are allowed symbols
comments - are comments...
name - is a variation name. [a-z A-Z 0-9 _] are allowed symbols
comments - are comments... don't forget space before #
-p or --parent - name of parent variation. If no name provided, "default" variation is the parent
"""
8 changes: 5 additions & 3 deletions python/ccdb/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.schema import Column, ForeignKey
from sqlalchemy.types import Integer, String, Text, DateTime, Enum, Boolean
from sqlalchemy.orm import reconstructor
from sqlalchemy.orm import reconstructor, relation
from sqlalchemy.orm import relationship, backref

Base = declarative_base()
Expand Down Expand Up @@ -221,7 +221,7 @@ def request(self):
def __repr__(self):
return "<Assignment '{0}'>".format(self.id)

def print_deps(self):
def print_info(self):
print " ASSIGNMENT: " + repr(self) \
+ " TABLE: " + repr(self.constant_set.type_table)\
+ " RUN RANGE: " + repr(self.run_range)\
Expand Down Expand Up @@ -263,6 +263,8 @@ class Variation(Base):
created = Column(DateTime, default=datetime.datetime.now)
comment = Column(Text)
author_id = Column('authorId', Integer, default=1)
parent_id = Column('parentId', Integer, ForeignKey('variations.id'), default=1)
parent = relation('Variation', remote_side=[id])

def __repr__(self):
return "<Variation {0} '{1}'>".format(self.id, self.name)
Expand Down Expand Up @@ -546,6 +548,6 @@ def list_to_table(data, col_count):
for row_i in range(row_count):
row = []
for col_i in range(col_count):
row.append(data[row_i*col_count + col_i])
row.append(data[row_i * col_count + col_i])
tabled_data.append(row)
return tabled_data
29 changes: 15 additions & 14 deletions python/ccdb/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
from sqlalchemy.orm.exc import NoResultFound
from sqlalchemy.sql.expression import desc
from .model import Directory, TypeTable, TypeTableColumn, ConstantSet, Assignment, RunRange, Variation, User, LogRecord
from ccdb.errors import DirectoryNotFound, TypeTableNotFound, RunRangeNotFound, DatabaseStructureError, UserNotFoundError, UserExistsError, \
from ccdb.errors import DirectoryNotFound, TypeTableNotFound, RunRangeNotFound, DatabaseStructureError, \
UserNotFoundError, UserExistsError, \
VariationNotFound

import table_file
Expand Down Expand Up @@ -51,9 +52,9 @@ def __init__(self):
self._auth = authentication.Authentication(self)
self._auth.current_user_name = "anonymous"
self.logging_enabled = True
self._no_structure_message = "No database structure found. Possibly you are trying to connect " +\
"to wrong SQLite file or to MySQL database without schema. " +\
"Original SqlAlchemy error is: " + os.linesep + os.linesep + "{0}"
self._no_structure_message = "No database structure found. Possibly you are trying to connect " + \
"to wrong SQLite file or to MySQL database without schema. " + \
"Original SqlAlchemy error is: " + os.linesep + os.linesep + "{0}"


#----------------------------------------------------------------------------------------
Expand Down Expand Up @@ -907,7 +908,7 @@ def search_variations(self, table_or_path, run=0, name=None, limit=0, offset=0):
#----------------------.--------------------------
# Create variation by name
#------------------------------------------------
def create_variation(self, name, comment=""):
def create_variation(self, name, comment="", parent_name=""):
"""
Create variation by name
Expand All @@ -923,8 +924,12 @@ def create_variation(self, name, comment=""):
variation = Variation()

if self.session.query(Variation).filter(Variation.name == name).count() > 0:
raise ValueError(
"Cannot create a new variation with name {0}. Variation with that name already exists".format(name))
raise ValueError("Cannot create a new variation with name {0}. "
"Variation with that name already exists".format(name))

if parent_name:
parent = self.get_variation(parent_name)
variation.parent = parent

variation.comment = comment
variation.name = name
Expand All @@ -936,7 +941,8 @@ def create_variation(self, name, comment=""):
self.create_log_record(user=user,
affected_ids=[variation.__tablename__ + str(variation.id)],
action="create",
description="Created variation '{0}'".format(variation.name),
description="Created variation '{0}' parent is '{1}'".format(variation.name,
variation.parent.name),
comment=variation.comment)

return variation
Expand Down Expand Up @@ -1367,7 +1373,7 @@ def create_user(self, name, pwd="", roles=[], user_info=""):
if user.is_deleted:
user.is_deleted = False
else:
raise UserExistsError("The user with name '"+name+"' exists in the database", name)
raise UserExistsError("The user with name '" + name + "' exists in the database", name)
except UserNotFoundError:
user = User()
user.last_action_time = datetime.now()
Expand All @@ -1393,11 +1399,6 @@ def delete_user(self, name):
self.session.commit()







def get_current_user(self):
"""
Expand Down
10 changes: 10 additions & 0 deletions python/tests/provider_fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,13 +281,23 @@ def test_variations(self):
v = self.provider.create_variation("abra_kozyabra")
self.assertIsNotNone(v)
self.assertNotEquals(v.id, 0)
self.assertEquals(v.parent_id, 1)
self.assertEquals(v.name, "abra_kozyabra")

# DELETE RUN-RANGE TEST
#----------------------------------------------------
self.provider.delete_variation(v)
self.assertRaises(VariationNotFound, self.provider.get_variation, "abra_kozyabra")

#Now create with comment and parent
v = self.provider.create_variation("abra_kozyabra", "Abra!!!", "test")
self.assertEquals(v.parent.name, "test")
self.assertEquals(v.comment, "Abra!!!")

#cleanup
self.provider.delete_variation(v)


def test_assignments(self):
"""Test Assignments"""
self.provider.connect(self.connection_str) # this test requires the connection
Expand Down
2 changes: 1 addition & 1 deletion python/tests/test_console_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def test_mk_rm_table(self):

def test_mk_rm_variation(self):
"""mkvar, rm. Create variation and delete it"""
self.context.process_command_line("mkvar auto_testing_variation")
self.context.process_command_line("mkvar auto_testing_variation -p test #hahaha")
#TODO check test table internals are right
self.context.process_command_line("rm --force -v auto_testing_variation")

Expand Down
2 changes: 1 addition & 1 deletion sql/ccdb.mysql.sql
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ CREATE TABLE IF NOT EXISTS `ccdb`.`variations` (
`description` VARCHAR(255) NULL,
`authorId` INT NOT NULL DEFAULT 1,
`comment` TEXT NULL DEFAULT NULL,
`parentId` INT NOT NULL DEFAULT 0,
`parentId` INT NOT NULL DEFAULT 1,
PRIMARY KEY (`id`),
UNIQUE INDEX `id_UNIQUE` (`id` ASC),
INDEX `name_search` USING HASH (`name` ASC),
Expand Down
Binary file modified sql/ccdb_eer_diagram.mwb
Binary file not shown.
2 changes: 1 addition & 1 deletion src/Library/Providers/SQLiteDataProvider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1200,7 +1200,7 @@ Assignment* ccdb::SQLiteDataProvider::GetAssignmentShort(int run, const string&
//If We have not found data for this variation, getting data for parent variation
if((assignment == NULL && selectedRows==0) && variation->GetParentDbId()!=0)
{
return GetAssignmentShort(run, path, time, variation->GetParent()->GetName());
return GetAssignmentShort(run, path, time, variation->GetParent()->GetName(), loadColumns);
}

if(assignment == NULL)
Expand Down
6 changes: 6 additions & 0 deletions src/Tests/test_NoMySqlUserAPI.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include "CCDB/Log.h"
#include "CCDB/CalibrationGenerator.h"

#include <map>

using namespace std;
using namespace ccdb;

Expand Down Expand Up @@ -171,6 +173,10 @@ TEST_CASE("CCDB/UserAPI/SQLite_CalibrationGenerator","Use universal generator to
REQUIRE(tabledValues.size()==2);
REQUIRE(tabledValues[0].size()==3);
REQUIRE(tabledValues[0][0]=="2.2");

//Error regression test for bug with absent column names. Filling map will test the column names are OK
vector<map<string, string> > tableMapValues;
REQUIRE_NOTHROW(result = sqliteCalib->GetCalib(tableMapValues, "/test/test_vars/test_table"));
}

SECTION("Get Assignment test", "Test all elements of getting data through get assignment")
Expand Down

0 comments on commit b1506ef

Please sign in to comment.