Lang_constants constant_name not unique?

system · April 16, 2014, 2:50pm

yehster wrote on Wednesday, April 16, 2014:

Is there a reason why we don’t have a uniqueness index on the constant_name in lang_constants?

It seems like the behavior of xl would be inconsistent if more than one entry exists for the same constant_name.

system · April 16, 2014, 3:04pm

sunsetsystems wrote on Wednesday, April 16, 2014:

I can’t think of one. Uniqueness should certainly be enforced one way or another.

Rod
http://www.sunsetsystems.com/

system · April 16, 2014, 5:08pm

blankev wrote on Wednesday, April 16, 2014:

I have been involved for a long time in Translations for OpenEMR.

Can you give a real world example for uniqueness? I do not see in translations spreadsheet and connections-software related what need to be changed.

system · April 16, 2014, 6:24pm

yehster wrote on Wednesday, April 16, 2014:

I’m proposing something akin to

ALTER TABLE lang_constants DROP INDEX cons_name , ADD UNIQUE INDEX cons_name (constant_name ASC);

This is an issue with regards to the database representation of information, and primarily concerns when people add custom/additional translation.

Adding a uniqueness constraint to the database would provide an additional check to make sure that duplicate constant’s don’t accidentally get defined when definitions are added using the interface in “Administration > Other > Language”.

If duplicates entries exist, then the results of the query in translations.inc.php

$sql="SELECT * FROM lang_definitions JOIN lang_constants ON " .
"lang_definitions.cons_id = lang_constants.cons_id WHERE " .
“lang_id=? AND constant_name = ? LIMIT 1”;

Could be unpredictable.

However with regards to the spreadsheet, when just the spreadsheet is loaded, all of the constants “should” be unique. However, if for some reason the same constant had two rows in the spreadsheet (error with the perl script somewhere, manual edits that were done incorrectly…) This extra constraint would lead to an error being generated when the spreadsheet was loaded, rather than the spreadsheet being loaded and translations potentially being unpredictable.

system · April 16, 2014, 6:28pm

yehster wrote on Wednesday, April 16, 2014:

I’m guessing that the perl scripts work to maintain uniqueness. Changing the index to unique would provide additional “protection” when manual edits are made

system · April 16, 2014, 6:33pm

sunsetsystems wrote on Wednesday, April 16, 2014:

Be sure to handle the case of upgrading a database where cons_name has non-unique values for some reason. You don’t want the upgrade script to fail.

Rod
http://www.sunsetsystems.com/

system · April 16, 2014, 6:35pm

blankev wrote on Wednesday, April 16, 2014:

What I understand is that there might be included duplicates in the spreadsheet. As far as I made translations, the only duplicates in a rows are the ones with a " " space at the end of a Constant and so also in Definitions. But translations wise for the used scripts or I can’t be of any help. For manual added translations, as long as a sentence/constant is translated there is no need for uniqueness. The translated words represent the Constant and the constant is used for interactions and not the translations. Correct me if I am wrong.

If by manual you mean to xl(…) translation characters as a part of the software this extra constant will be included or not included if needed due to not yet included in the Constant column.

system · April 16, 2014, 6:37pm

yehster wrote on Wednesday, April 16, 2014:

Except if there are non-unique constants, then there isn’t really a good “automatic” way to resolve conflicts.

system · April 16, 2014, 6:39pm

yehster wrote on Wednesday, April 16, 2014:

You are wrong.
Both the constant and the definition are used in the lookup process.

system · April 16, 2014, 6:43pm

blankev wrote on Wednesday, April 16, 2014:

You having said this, I am out! Please let’s hope other developers will jump in to support the idea of uniqueness. I did translate some different constants but were definitely in luck not to encounter problems due to uniqueness.

system · April 16, 2014, 6:52pm

yehster wrote on Wednesday, April 16, 2014:

Pieter, thanks for your interest and desire to help.

The basic issue in my mind is that the table should have been enforcing uniqueness all along. If all that people are using is just the spreadsheet, this is a non-issue, but if someone is trying to use definitions from additional sources, there may be a problem.

system · April 16, 2014, 6:53pm

sunsetsystems wrote on Wednesday, April 16, 2014:

I think you might be able to with a couple of messy queries using subqueries. One to update lang_definitions.cons_id to point to just one of the duplicates, and another to remove the duplicated constants. I’d have to play with it for a bit.

Then you might be left with some duplicated (lang_id, cons_id) pairs in the lang_definitions table, for which the same problem of possible duplicates exists.

Nothing is easy.

Rod
http://www.sunsetsystems.com/

system · April 16, 2014, 6:56pm

yehster wrote on Wednesday, April 16, 2014:

Except there may also be conflict resolution needed for when multiple lang_definitions exist that might map to the same constant_name with different definitions. Which definition should win?

I am certain that appropriate queries are possible, but to make them “one-size fits all” would be difficult.

system · April 16, 2014, 7:09pm

sunsetsystems wrote on Wednesday, April 16, 2014:

It doesn’t really matter which one wins, since the translation lookup will not have a predictable result anyway. Throwing either one away will be better than doing nothing.

Yes it’s kinda messy but I suspect do-able. Main thing is to make sure upgrading is robust.

Rod
http://www.sunsetsystems.com/

system · April 16, 2014, 7:31pm

blankev wrote on Wednesday, April 16, 2014:

The only manual problem that exists than is your additional translations through Administration => Others -> Translations etc…

But these new translations have to be done in the software …php …txt(?) …sql files, etc. and after the xl(…) is added give it the translation definition. This is like creating a personal local translation-file.

BTW for translation there is used: xl(…) xl t(…) xl a(…) I did not check for completeness, but there seemed to be a reason for this different translation options.

system · April 16, 2014, 7:38pm

yehster wrote on Wednesday, April 16, 2014:

xlt and xla are wrappers for the base xl function.

An explanation of the purpose/function is available on the wiki.

http://www.open-emr.org/wiki/index.php/Codebase_Security#SQL-Injection_and_Cross-Scripting_Prevention

and also

github.com

openemr/openemr/blob/master/library/htmlspecialchars.inc.php#L103


function hsc_private_xl_or_warn($key)
{
   if (function_exists('xl')) {
       return xl($key);
   } else {
       trigger_error(
           'Translation via xl() was requested, but the xl()'
           . ' function is not defined, yet.',
           E_USER_WARNING
       );
       return $key;
   }
}


/**
* Translate via xl() and then escape via text().
*
* @param string $key The string to escape, possibly including "&", "<",
*                    or ">".
* @return string The string, with "&", "<", and ">" escaped.
*/

system · April 16, 2014, 8:04pm

blankev wrote on Wednesday, April 16, 2014:

- See more at: http://www.open-emr.org/wiki/index.php/Codebase_Security#SQL-Injection_and_Cross-Scripting_Prevention

White screen of death… never had reconned this was an option, but might explain some forums on white screen.

system · April 17, 2014, 3:34am

bradymiller wrote on Thursday, April 17, 2014:

Hi,

This is a “unique” situation where you may not need to spend much time worrying about upgrading the table. Could just place it in the database.sql file and then when users upgrade to the next official translation set, can fix it here:

github.com

openemr/openemr/blob/master/contrib/util/language_translations/buildLanguageDatabase.pl#L477


$tempCounter += 1; 


# create header
my $return = "\
-- 
-- Table structure for table `lang_constants`
-- 
\n
DROP TABLE IF EXISTS `lang_constants`;
CREATE TABLE `lang_constants` (
 `cons_id` int(11) NOT NULL auto_increment,
 `constant_name` mediumtext BINARY,
 UNIQUE KEY `cons_id` (`cons_id`),
 KEY `constant_name` (`constant_name`(100))
) ENGINE=InnoDB AUTO_INCREMENT=".$tempCounter." ;
\n
-- 
-- Dumping data for table `lang_constants`
--\n\n";


# insert table input

For testing:
-Ensure a new install works.
-Will want to test to ensure funny things(ie. especially a crash) don’t happen when bringing stuff in from lang_custom if for some reason there are two items with same constant lingering in there.

Also, the perl scripts that build the translation spreadsheet are set up to not allow identical constants:

github.com

openemr/openemr/blob/master/contrib/util/language_translations/collectConstants.pl

#!/usr/bin/perl
#
# 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
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
#
# author Brady Miller
# email  brady.g.miller@gmail.com
# date   03/25/09
#
# This is a perl script that will collect unique constants within
# OpenEMR source code.
#  It effectively finds all xl("constants","") within OpenEMR.
#  It will filter out constants found in manuallyRemovedConstants.txt
#  It will add constants found in (ensure not repeated) manuallyAddedConstants.txt 
#  It can also compare to a previous list to find new constants.
#  
#  Example commands:
#

This file has been truncated. show original

github.com

openemr/openemr/blob/master/contrib/util/language_translations/combineConstantsSpreadsheet.pl

#!/usr/bin/perl
#
# 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
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
#
# author Brady Miller
# email  brady.g.miller@gmail.com
# date   04/03/09
#
# This is a perl script that will create a new ranslation spreadsheet
# file from a constant file and an old spreadsheet file.  Output will
# go to newSpreadsheet.tsv and errors and message swill be logged to
# log.txt.
#
#  Example commands:
#
#  -Below will output the new spreadsheet:
#  ./commonConstantsSpreadsheet.pl spreadsheet.tsv constants.txt

This file has been truncated. show original

The perl script that builds the database tables from the spreadsheet does not explicitly ensure that(but is uses the spreadsheet built from above scripts and also does confirmations of constants to the official partner constant list):

github.com

openemr/openemr/blob/master/contrib/util/language_translations/buildLanguageDatabase.pl

#!/usr/bin/perl
#
# Copyright (C) 2009-2013 Brady Miller <brady.g.miller@gmail.com>
#
# 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
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
#
# Author   Brady Miller <brady.g.miller@gmail.com>
# Author   Ramin Moshiri <raminmoshiri@gmail.com> 
# 
# This is a perl script that will build the language translation sql
# dumpfiles from the tab delimited language translation spreadsheet.
# It will create two output dumpfiles:
#   languageTranslations_utf8.sql
#   languageTranslations_latin1.sql (minus utf languages)
# It will also validate the spreadsheet and create a new spreadsheet
# that can be used for further downstream editing and re-importing
# back into Google Docs. It also outputs a logfile log.txt with

This file has been truncated. show original

-brady
OpenEMR

system · April 17, 2014, 3:39am

bradymiller wrote on Thursday, April 17, 2014:

A very important item I forget to mention here; the change to database.sql is actually going to do nothing(it’s good to do for clarity sake), since when it brings in the translation set on install, tables will get replaced by:

github.com

openemr/openemr/blob/master/contrib/util/language_translations/currentLanguage_utf8.sql


--
-- Ensure correct encoding
--
SET NAMES utf8;


--
-- Table structure for table `lang_languages`
--


DROP TABLE IF EXISTS `lang_languages`;
CREATE TABLE `lang_languages` (
  `lang_id` int(11) NOT NULL auto_increment,
  `lang_code` char(2) NOT NULL default '',
  `lang_description` varchar(100) default NULL,
  `lang_is_rtl` TINYINT DEFAULT 0,
  UNIQUE KEY `lang_id` (`lang_id`)
) ENGINE=InnoDB AUTO_INCREMENT=54 ;

This file has been truncated. show original

So, for testing, remember to change it there.
-brady

system · April 17, 2014, 3:44am

bradymiller wrote on Thursday, April 17, 2014:

Also,
While looking at the tables, doesn’t it seem odd that constant name is limited to 255 while the definition is a mediumtext?
-brady