Derby
  1. Derby
  2. DERBY-400

Network client message strings not internationalized

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.1.1.0
    • Fix Version/s: 10.2.1.6
    • Component/s: Network Client
    • Labels:
      None

      Description

      In investigating DERBY-254, I discovered that all SQLExceptions are thrown with hardcoded English error messages. This needs to be modified to use message string resource bundles similar to the embedded code.

      1. DERBY-400.diff
        868 kB
        David Van Couvering

        Issue Links

        1.
        Create internationalization framework for network client Sub-task Closed David Van Couvering
         
        2.
        Internationalize messages CallableStatement40 to Cursor.java in org.apache.derby.client.am Sub-task Closed David Van Couvering
         
        3.
        International messages DatabaseMetaData to GetFileInputStreamAction in org.apache.derby.client.am Sub-task Closed David Van Couvering
         
        4.
        Internationalize messages in GetResourceBundleAction to ParameterMetadata in org.apache.derby.client.am Sub-task Closed David Van Couvering
         
        5.
        Internationalize messages in PreparedStatement to Section in org.apache.derby.client.am Sub-task Closed David Van Couvering
         
        6.
        Internationalize messages in SectionManager to XaException in org.apache.derby.client.am Sub-task Closed David Van Couvering
         
        7.
        Internationalize all classes in org.apache.derby.client top-level package Sub-task Closed David Van Couvering
         
        8.
        Internationalize CcsidManager to FdocaSimpleArray in org.apache.derby.client.net Sub-task Closed David Van Couvering
         
        9.
        Internationalze NetAgent to NetDatabaseMetadata40 in org.apache.derby.client.net Sub-task Closed David Van Couvering
         
        10.
        Internationalize NetIndoubtTransaction to NetSqlca in org.apache.derby.client.net Sub-task Closed David Van Couvering
         
        11.
        Internationalize NetSqldta to OpenSocketAction in org.apache.derby.client.net Sub-task Closed David Van Couvering
         
        12.
        Internationalize Reply to Typdef in org.apache.derby.client.net Sub-task Closed David Van Couvering
         
        13.
        Internationalize all classes in java/client/org/apache/derby/jdbc Sub-task Closed David Van Couvering
         
        14.
        Remove org.apache.derby.client.resource package - no longer needed Sub-task Closed David Van Couvering
         
        15.
        Convert SqlException to no longer extend SQLException Sub-task Closed David Van Couvering
         
        16.
        Internationalize SqlWarning and its usages Sub-task Closed David Van Couvering
         
        17.
        Move all client messages to messages.properties Sub-task Closed David Van Couvering
         
        18.
        Add a test to i18n that verifies that there are no duplicate message ids in messages.properties Sub-task Closed Unassigned
         
        19.
        Document user-visible changes affected by internationalization of the network client Sub-task Closed David Van Couvering
         
        20.
        Remove old constructors for SqlException and DisconnectException and clean up any remaining users of the old constructors Sub-task Closed David Van Couvering
         

          Activity

          Hide
          David Van Couvering added a comment -

          Internationalizing error messages can't be done without common code framework without a lot of code duplication.

          Show
          David Van Couvering added a comment - Internationalizing error messages can't be done without common code framework without a lot of code duplication.
          Hide
          David Van Couvering added a comment -

          I am attaching the initial changes I have made for this. This includes the i18n framework for the client and the conversion of a few exceptions along with the associated unit test. THIS IS NOT FOR INCLUSION IN THE PROJECT. I am posting this for review to make sure I am on the right track and there are no concerns before I go ahead and convert a bunch of error messages.

          Things to note:

          • As Dan suggested, I am building a new i18n framework rather than trying to create a common jar file
          • I am adding support for SQL States at the same time by using the SQL State as the message id for the message properties file. This is the same model as in the engine
          • I am using "XN" for implementation-defined SQL State to indicate client SQL states
          • I am using standard SQL States where they apply
          • I am using static methods to "encapsulate" messages with the appropriate parameter list. This is the same approach as used in the tools
          • The LocalizedResource class is a cut-and-paste from the tools directory, with a lot of code that did not apply removed (code used for formatting, etc., by ij)

          Your comments are much appreciated; I do hope to avoid making a lot of changes and then realizing my approach was flawed in some way. So far it looks to be working pretty well...

          Show
          David Van Couvering added a comment - I am attaching the initial changes I have made for this. This includes the i18n framework for the client and the conversion of a few exceptions along with the associated unit test. THIS IS NOT FOR INCLUSION IN THE PROJECT. I am posting this for review to make sure I am on the right track and there are no concerns before I go ahead and convert a bunch of error messages. Things to note: As Dan suggested, I am building a new i18n framework rather than trying to create a common jar file I am adding support for SQL States at the same time by using the SQL State as the message id for the message properties file. This is the same model as in the engine I am using "XN" for implementation-defined SQL State to indicate client SQL states I am using standard SQL States where they apply I am using static methods to "encapsulate" messages with the appropriate parameter list. This is the same approach as used in the tools The LocalizedResource class is a cut-and-paste from the tools directory, with a lot of code that did not apply removed (code used for formatting, etc., by ij) Your comments are much appreciated; I do hope to avoid making a lot of changes and then realizing my approach was flawed in some way. So far it looks to be working pretty well...
          Hide
          David Van Couvering added a comment -

          I am attaching a patch to show the work in progress. In particular, I discovered that there were some exceptions that had the same SQL State but different message texts. As a result I constructed the MessageId class, which encapsulates a specific message id and lets you have unique message ids with the same SQL state. This encapsulation also has other advantages: you can detect duplicate use of the same id, and test code can ensure that all message ids declared are actually tested.

          I also am defining the SQLException error code from network client exceptions to be more in line with the error codes coming from the server. I did this by adding a new value to org.apache.derby.engine.iapi.error.ExceptionSeverity.java called NETWORK_CLIENT_SEVERITY. In the past this error code was hardcoded to -99999.

          I would appreciate any feedback.with this approach before I forge ahead too much farther. I would also like to suggest that, after running full tests, I take a snapshot and get it checked in. These changes should be fully backward compatible; the only visible change is that some SQL Exceptions will have a SQL state defined now, while others will still have a null SQL State.

          Show
          David Van Couvering added a comment - I am attaching a patch to show the work in progress. In particular, I discovered that there were some exceptions that had the same SQL State but different message texts. As a result I constructed the MessageId class, which encapsulates a specific message id and lets you have unique message ids with the same SQL state. This encapsulation also has other advantages: you can detect duplicate use of the same id, and test code can ensure that all message ids declared are actually tested. I also am defining the SQLException error code from network client exceptions to be more in line with the error codes coming from the server. I did this by adding a new value to org.apache.derby.engine.iapi.error.ExceptionSeverity.java called NETWORK_CLIENT_SEVERITY. In the past this error code was hardcoded to -99999. I would appreciate any feedback.with this approach before I forge ahead too much farther. I would also like to suggest that, after running full tests, I take a snapshot and get it checked in. These changes should be fully backward compatible; the only visible change is that some SQL Exceptions will have a SQL state defined now, while others will still have a null SQL State.
          Hide
          David Van Couvering added a comment -

          This patch includes the following changes. I know I have the karma to check this in directly, but I would like it to be reviewed if someone has the inclination. I am in the middle of running tests. I can check it in once tests pass and the review comments have been addressed.

          • Based on Dan's review comments for the last incarnation of this patch, org.apache.derby.client.am.SqlException no longer extends java.sql.SQLException in preparation for JDBC4 support.
          • All public JDBC methods now throw SQLException instead of SqlException. They generally catch SqlException and convert this to SQLException using a utility routine on SqlException. This was a lot of work and the diff is quite large.
          • Some internal methods call public JDBC methods. These methods need to continue throwing SqlException or you end up with the majority of internal code having to handle and/or throw SQLException. So these internal methods take SQLExceptions and wrap them in a SqlException. The underlying SQLException is then "unwrapped" prior to throwing the exception to the application code
          • I have added new constructors on SqlException that take a message id instead of a hardcoded string. I use a simple wrapper class, MessageId, to hold the message id, otherwise the old constructors for SqlException have matching signatures and the compiler complains (as well it should). This wrapper class can be refactored out once the i18n conversion is complete.
          • I have added code in org.apache.derby.shared.common that provides utility routines for i18n and exception management. This code is currently only used by the client side.
          • I have migrated about a dozen client-side messages to be internationalized
          Show
          David Van Couvering added a comment - This patch includes the following changes. I know I have the karma to check this in directly, but I would like it to be reviewed if someone has the inclination. I am in the middle of running tests. I can check it in once tests pass and the review comments have been addressed. Based on Dan's review comments for the last incarnation of this patch, org.apache.derby.client.am.SqlException no longer extends java.sql.SQLException in preparation for JDBC4 support. All public JDBC methods now throw SQLException instead of SqlException. They generally catch SqlException and convert this to SQLException using a utility routine on SqlException. This was a lot of work and the diff is quite large. Some internal methods call public JDBC methods. These methods need to continue throwing SqlException or you end up with the majority of internal code having to handle and/or throw SQLException. So these internal methods take SQLExceptions and wrap them in a SqlException. The underlying SQLException is then "unwrapped" prior to throwing the exception to the application code I have added new constructors on SqlException that take a message id instead of a hardcoded string. I use a simple wrapper class, MessageId, to hold the message id, otherwise the old constructors for SqlException have matching signatures and the compiler complains (as well it should). This wrapper class can be refactored out once the i18n conversion is complete. I have added code in org.apache.derby.shared.common that provides utility routines for i18n and exception management. This code is currently only used by the client side. I have migrated about a dozen client-side messages to be internationalized
          Hide
          David Van Couvering added a comment -

          The SQL States will be fixed as part of internationalizing the client messages

          Show
          David Van Couvering added a comment - The SQL States will be fixed as part of internationalizing the client messages
          Hide
          David Van Couvering added a comment -

          For those of you working on internationalizing the network client messages, you need to take care when choosing a SQL State for a new client messages (e.g. one that is not duplicated or closely duplicated in the engine's messages.properties file).

          In most cases you can use "XN" which means a vendor-specific SQL State. However, if the error matches a predefined SQL State (either a class, subclass, or entire state) in the SQL2003 spec, you should use that. For example, an exception that affects the connection should use the prefix 08. A connection failure must be exactly 08006, connection does not exist 08003. The SQL State definitions can be found in section 23.1 of the SQL 2003 spec.

          Somebody correct me if I'm missing something, but there is also an entire set of SQLStates required by DRDA which (in general) are currently not being set by the client code. These SQLStates are defined in Chapter 8 (Volume 1) of the DRDA spec. We will need to set the SQL State appropriately based on this spec in the org/apache/derby/client/net classes. I know this code is hard to parse through if you're new to DRDA, so please do send out email if you need help/guidance.

          Note that doing this as correctly as possible is important for portability and also for future JDBC4 work where you create the appropriate SQLException subclass based on the SQL State...

          Thanks,

          David

          Show
          David Van Couvering added a comment - For those of you working on internationalizing the network client messages, you need to take care when choosing a SQL State for a new client messages (e.g. one that is not duplicated or closely duplicated in the engine's messages.properties file). In most cases you can use "XN" which means a vendor-specific SQL State. However, if the error matches a predefined SQL State (either a class, subclass, or entire state) in the SQL2003 spec, you should use that. For example, an exception that affects the connection should use the prefix 08. A connection failure must be exactly 08006, connection does not exist 08003. The SQL State definitions can be found in section 23.1 of the SQL 2003 spec. Somebody correct me if I'm missing something, but there is also an entire set of SQLStates required by DRDA which (in general) are currently not being set by the client code. These SQLStates are defined in Chapter 8 (Volume 1) of the DRDA spec. We will need to set the SQL State appropriately based on this spec in the org/apache/derby/client/net classes. I know this code is hard to parse through if you're new to DRDA, so please do send out email if you need help/guidance. Note that doing this as correctly as possible is important for portability and also for future JDBC4 work where you create the appropriate SQLException subclass based on the SQL State... Thanks, David
          Hide
          David Van Couvering added a comment -

          By the way, the SQL 2003 spec states that there is a specific SQL State class for Remote Database Access, which is "HZ". It refers to table 33, "SQLState Class Codes for RDA", which is a zen moment, because the table is empty.

          Can anyone explain the relationship between the "HZ" class for RDA and the SQL States defined in the DRDA spec, which do not make any reference or use of the "HZ" class?

          Thanks,

          David

          Show
          David Van Couvering added a comment - By the way, the SQL 2003 spec states that there is a specific SQL State class for Remote Database Access, which is "HZ". It refers to table 33, "SQLState Class Codes for RDA", which is a zen moment, because the table is empty. Can anyone explain the relationship between the "HZ" class for RDA and the SQL States defined in the DRDA spec, which do not make any reference or use of the "HZ" class? Thanks, David
          Hide
          Kathey Marsden added a comment -

          I was wondering if there was a document describing the new SQLStates and SQLState changes that are being made as part of this effort that users could use in preparation for the change.

          Show
          Kathey Marsden added a comment - I was wondering if there was a document describing the new SQLStates and SQLState changes that are being made as part of this effort that users could use in preparation for the change.
          Hide
          David Van Couvering added a comment -

          A document does not currently exist, but I can write something up, that would be good. Can you describe what it is you think would be valuable – not only in terms of content but also in terms of layout? That would be really helpful.

          I'm assuming this should be part of the official documentation, not just a Wiki page, right?

          Show
          David Van Couvering added a comment - A document does not currently exist, but I can write something up, that would be good. Can you describe what it is you think would be valuable – not only in terms of content but also in terms of layout? That would be really helpful. I'm assuming this should be part of the official documentation, not just a Wiki page, right?
          Hide
          Kathey Marsden added a comment -

          For the documentation I would guess this page would be updated:
          http://db.apache.org/derby/docs/10.1/ref/rrefexcept71493.html
          and that would be a great start.

          But in addition I am looking for something that would help users prepare to move to 10.2 so would separate out the new SQLStates, any changes that were made that were not just a move from a null SQLState to a new SQLState, any error code changes etc. A Wiki page would be fine for that. Ultimately I think some rendition of it would go in the release notes.

          Kathey

          Show
          Kathey Marsden added a comment - For the documentation I would guess this page would be updated: http://db.apache.org/derby/docs/10.1/ref/rrefexcept71493.html and that would be a great start. But in addition I am looking for something that would help users prepare to move to 10.2 so would separate out the new SQLStates, any changes that were made that were not just a move from a null SQLState to a new SQLState, any error code changes etc. A Wiki page would be fine for that. Ultimately I think some rendition of it would go in the release notes. Kathey
          Hide
          David Van Couvering added a comment -

          Resolved, all sub-tasks completed.

          Show
          David Van Couvering added a comment - Resolved, all sub-tasks completed.

            People

            • Assignee:
              David Van Couvering
              Reporter:
              David Van Couvering
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development