12 Replies Latest reply on Feb 7, 2007 12:46 PM by Clebert Suconic

    Duplicated client IDs

    Clebert Suconic Master

      I'm working on the integration testsuite, and one failing test is DuplicateClientIDUnitTestCase::testDuplicate.

      I wrote a similar test within JBossMessaging testsuite (org.jboss.test.jbossmessaging.test.DuplicateClientIDUnitTestCase) and the test is failing.

      If we decide this is not a bug, we will have to decide what to do on the integration testsuite.

      This is the JIRA issue:

      http://jira.jboss.org/jira/browse/JBMESSAGING-791

        • 1. Re: Duplicated client IDs
          Tim Fox Master

          I think this bug is a duplicate of http://jira.jboss.com/jira/browse/JBMESSAGING-286.

          It looks like an easy fix, you could do it now if you want.

          • 2. Re: Duplicated client IDs
            Clebert Suconic Master

            I just looked (by accident though) on DuplicateClientIDTest::testPreconfiguredDuplicateClientID and the test is wrong.

            A preConfiguredDuplicateClientID is failing... but we don't have the user properly configured.

            So... I need a way to use setup to pre-configure an user's pre-configured clientID. Any ideas on how I could do that?

            • 3. Re: Duplicated client IDs
              Clebert Suconic Master

              I mean... it would be nice if I could call a MBean operation to setUsersDefaultClientId somewhere on the testsuite.

              We could add something to ServerManager.. true... but I was thinking about something that could be reused into integration testsuite also.

              • 5. Re: Duplicated client IDs
                Tim Fox Master

                You just need to add the users in the sql properties of JDBCJMSUserManager

                • 6. Re: Duplicated client IDs
                  Tim Fox Master

                  See JDBCJMSUserManager::insertUserRoleData()

                  • 7. Re: Duplicated client IDs
                    Clebert Suconic Master

                    I had to add a new method to JDBCSupport to act as a hook.

                    JDBCSupport is doing a crosscheck betwen sqlProperties and default statements. When you define POPULATE.TABLES... JDBCSupport throws an exception during start.

                    protected boolean ignoreVerificationOnStartup(String statementName)
                     {
                     // Do not cross-check on POPULATE.TABLES. as we just load the tables with them
                     if (statementName.startsWith("POPULATE.TABLES."))
                     {
                     return true;
                     }
                     return false;
                     }






                    • 8. Re: Duplicated client IDs
                      Clebert Suconic Master

                       

                      "timfox" wrote:
                      You just need to add the users in the sql properties of JDBCJMSUserManager


                      I will add a new user to the MockSecurityManager.

                      And I would need to add an user to mysql-persistence-service.xml.

                      I mean... I don't think it would be such a good to add a test user to mysql-persistence-service.xml, as that file is also distributed with the release. I guess JBossMQ does distribute those test users, but I'm not sure if we should do the same.

                      I have the same issue with the integration testsuite... JBossMQ has those users pre-configured out of the box.




                      • 9. Re: Duplicated client IDs
                        Tim Fox Master

                        Clebert - why have you added a new query:

                         SELECT_EXISTS_REF=SELECT MESSAGEID FROM JMS_MESSAGE_REFERENCE WHERE CHANNELID = ? AND MESSAGEID = ?
                        +SELECT_EXISTS_REF_MESSAGEID=SELECT MESSAGEID FROM JMS_MESSAGE_REFERENCE WHERE MESSAGEID = ?
                        


                        There's one already there, please use that.

                        You need the channel id anyway. Just query for the message id without specifying the channel id means nothing.

                        • 10. Re: Duplicated client IDs
                          Tim Fox Master

                          Also, some more comments.

                          I don't like the naming of the parameter "retry". Why retry? How about "checkForDuplicates" or "check"?

                          When sending a transaction ans check=true, there is no need to check if every reference exists in the database, you only need to check one of them.

                          After all, it is a transaction. If the first one made it, they all made it. This is inefficient otherwise.

                          • 11. Re: Duplicated client IDs
                            Clebert Suconic Master

                             

                            "timfox" wrote:
                            Clebert - why have you added a new query:

                             SELECT_EXISTS_REF=SELECT MESSAGEID FROM JMS_MESSAGE_REFERENCE WHERE CHANNELID = ? AND MESSAGEID = ?
                            +SELECT_EXISTS_REF_MESSAGEID=SELECT MESSAGEID FROM JMS_MESSAGE_REFERENCE WHERE MESSAGEID = ?
                            


                            There's one already there, please use that.

                            You need the channel id anyway. Just query for the message id without specifying the channel id means nothing.


                            I don't have the old channelID on send, and besides I'm testing if the message was created on the old channel or on the current channel. So.. on that case I'm just checking if the message was already created on whatever channel.

                            • 12. Re: Duplicated client IDs
                              Clebert Suconic Master

                               

                              "Tim Fox" wrote:
                              Also, some more comments.

                              I don't like the naming of the parameter "retry". Why retry? How about "checkForDuplicates" or "check"?


                              Sure... check is a better name.. I will rename it.


                              "Tim Fox" wrote:

                              When sending a transaction ans check=true, there is no need to check if every reference exists in the database, you only need to check one of them.

                              After all, it is a transaction. If the first one made it, they all made it. This is inefficient otherwise.


                              I will add an extrat check on sendTransaction.. I was just relaying on the check that was already being called on send.