Menu

dbunit Merge Request #43: Allow users to customize ReplacementDataSet behavior. (rejected)

Merging...

Merged

Something went wrong. Please, merge manually

Checking if merge is possible...

Something went wrong. Please, merge manually

Iwao AVE! wants to merge 1 commit from /u/kforkiss/dbunit/ to master, 2019-03-16

Hi team,

The current ReplacementDataSet is not powerful enough for complex requirements.
The one that I wanted to achieve, in particular, was to set a date or timestamp relative to the current date.
For example, "${TODAY+1}" or "${TODAY-30 10:00:00}".

There are a few old tickets on the tracker, but there seems to be no progress, so far (sorry if I missed it).
https://sourceforge.net/p/dbunit/feature-requests/138/
https://sourceforge.net/p/dbunit/feature-requests/152/

This patch provides users great flexibility while keeping backward compatibility.

There are two new interfaces: IValueMatcher and IValueReplacer.

  • IValueMatcher is used to check if there is a registered replacer that can handle the value.
  • IValueReplacer performs the actual replacement (the name is John Hurst's idea).

Users can use the existing addReplacementObject() method to register implementations of these new interfaces.
e.g.

dataSet.addReplacementObject(new CustomMatcher(), new CustomReplacer());

For convenience, it's also possible to pass a regex pattern as the first argument (built-in RegexValueMatcher is registered internally).
e.g.

dataSet.addReplacementObject("__TODAY[+-][0-9]+__", new RelativeDateReplacer());

The patch is relatively small, so please see the diff for the details.
There should be docs and more tests, but I would like to see if the team likes it or not before spending more time.

Thank you,
Iwao

Commit Date  
[9e12e1] (replacementdataset-enhancement) by Iwao AVE! Iwao AVE!

Allow users to customize ReplacementDataSet behavior.

2019-01-23 14:26:06 Tree

Discussion

  • Jeff Jensen

    Jeff Jensen - 2019-01-25

    Hi,
    I'm not following what is not possible with the current implementation. For example, I've used
    [TODAY], [TOMORROW], and many more to handle relative dates/times, e.g. populateReplacementObjects() example has a few on http://dbunit.sourceforge.net/testcases/PrepAndExpectedTestCase.html :

        private void populateReplacementObjects()
        {
            replacementObjects.put("[IGNORE]", null);
            replacementObjects.put("[NULL]", null);
            replacementObjects.put("[TIMESTAMP_TODAY]", TestDatabaseDates.TIMESTAMP_TODAY);
            replacementObjects.put("[TIMESTAMP_TOMORROW]", TestDatabaseDates.TIMESTAMP_TOMORROW);
            replacementObjects.put("[TIMESTAMP_YESTERDAY]", TestDatabaseDates.TIMESTAMP_YESTERDAY);
        }
    
     
  • Iwao AVE!

    Iwao AVE! - 2019-01-25

    Thank you for reviewing this, Jeff !

    It is possible, for sure, but it is quite cumbersome if one needs dozens of those dates/timestamps.
    In [#138] , mattias g wrote that this feature "avoids redundant code and improves reusability".
    In [#152] , John Hurst wrote "The current ReplacementDataSet is too limited. It's too hardcoded around strings and maps of replacements."
    And I completely agree with them.
    Wouldn't you?

    Many developers seemed to expect the dynamic approach as well.
    http://dbunit.996259.n3.nabble.com/Can-you-somehow-use-variable-date-values-within-dataset-XML-td373.html
    https://stackoverflow.com/questions/15925979
    https://merereflections.wordpress.com/2010/08/16/dbunit-and-setting-up-changing-data/

    This user ended up creating a custom dataset.
    http://musingsofaprogrammingaddict.blogspot.com/2009/02/scriptable-data-set-for-dbunit.html
    (BTW, the same JSR-223 scripting can be implemented with this enhancement probably.)

    As a non-date example, instead of declaring each instance, users may be able to write ${clob:/path/to/resource} in dataset to load CLOB data.
    https://stackoverflow.com/questions/28372160/

    Hope this explains the background of this request.

     

    Related

    Feature Requests: #138
    Feature Requests: #152

    • Jeff Jensen

      Jeff Jensen - 2019-01-25

      Hope this explains the background of this request.

      Thank you for the time, code, info... let's discuss further, I hope you don't mind and have the time to.

      it is quite cumbersome if one needs dozens of those dates/timestamps.

      Yes, with a lot of them it becomes a big list of mappings, even though they are simple. However, a couple of benefits of the current approach are:

      • Centralized definitions in one place so only one place to change for all uses and anyone knows which ones are available instead of spread throughout the data files as with the proposal.
      • A declarative approach (tokens in the data files) instead of imperatively coding it in all the data files.

      In [#138] , mattias g wrote that this feature "avoids redundant code and improves reusability".

      He also wrote "we should also rewrite the internals of the old ReplacementDataSet to use this new functionality." Is this your proposal for the next step? How far will you take this new feature, including doc updates and tests?

      In [#152] , John Hurst wrote ...

      Is there anything from John's ideas that are not possible with your implementation?

      And I completely agree with them.
      Wouldn't you?

      Maybe; I don't rush into it because:

      • I've used dbUnit a lot and for a very long time and have not had need for this change (but doesn't mean shouldn't do), so before adding/changing a significant feature, we want to ensure it is the best approach we can design.
      • Enabling "dynamic data" can lead to unreproducible/inconsistent tests and results. Before committing such changes, we need to review/analyze scenarios to ensure we're ok with potential negative consequences.
      • The current solution and "new ideas" all involve placing a string in the dbUnit file field and having it interpret it, currently as a replacement token only and proposal also as a function call. The theory of the dynamic function call field is great, but I haven't yet seen practicality or true need for it that's not already handled. Poor support and confusion can result from multiple features doing same thing.

      Many developers seemed to expect the dynamic approach as well.

      Except that all of those examples are easily accomplished with the current ReplacementDataSet feature, nothing else needed. While someone wanted to imperatively write "now - 14 days" in a date field, defining it to a ReplacementDataSet and declaring it in the data file is easy. I wonder if the authors of those examples/changes didn't understand the feature well?

      JSR-223 scripting

      Please test if this is also possible with your proposal.

      ${clob:/path/to/resource} in dataset to load CLOB data.

      Yes, a generic approach to add any content type in the specific field is useful and important.
      Unless I misunderstand something, supporting multiple data types is different from dynamic/calling a script for a field (the latter is a "how").

      dbUnit already has the DataType feature, and blobs is well documented: http://dbunit.sourceforge.net/datatypes.html#blob
      Is there a way (and if there is, is it a good idea) to relate your proposed change to the DataType feature and build on the existing vs a completely orthogonal new feature?

       

      Related

      Feature Requests: #138
      Feature Requests: #152

      • Iwao AVE!

        Iwao AVE! - 2019-01-27

        Hi again,

        However, a couple of benefits of the current approach are:

        I understand the benefits and am not denying it, but I still believe that there should be a way to skip the redundancy.
        In my case, most dates/timestamps have no symbolic meaning and are used only in one place, so there is little benefit in declaring/referencing tokens.

        He also wrote "we should also rewrite the internals of the old ReplacementDataSet to use this new functionality." Is this your proposal for the next step?

        Kyle's patch introduced a new dataset, but my patch modifies ReplacementDataSet in non-intrusive manner, so I have no plan for further modification.

        How far will you take this new feature, including doc updates and tests?

        My English is not great, but I'll try my best once we reached an agreement. :)

        Is there anything from John's ideas that are not possible with your implementation?

        'script:' should not be a problem.
        And no one would write a custom IValueReplacer for 'blob:' because DbUnit already supports various sources as you pointed out.

        Maybe; I don't rush into it because:

        You should never rush to add a new feature! :)

        Unless I misunderstand something, supporting multiple data types is different from dynamic/calling a script for a field (the latter is a "how").

        After reading the SO answer, I assumed that it was possible to replace a token with a CLOB object for somehow (the answer didn't include the code instantiating 'YourClobObject').
        But if that is not the case, supporting 'clob:' with this enhancement might be difficult and I must apologize about my misunderstanding (my focus was/is on date and timestamp).

        dbUnit already has the DataType feature, and blobs is well documented: http://dbunit.sourceforge.net/datatypes.html#blob
        Is there a way (and if there is, is it a good idea) to relate your proposed change to the DataType feature and build on the existing vs a completely orthogonal new feature?

        It seems like a better solution indeed!
        I will read the BLOB implementation and see if it's possible to provide the similar flexibility to Date, Time and Timestamp.
        Off the top of my head, it would be great if syntax like the following were supported out-of-the-box.

        • [+1d] = date
        • [+30d] 10:30:00 = timestamp
        • [-1d](-1h) = timestamp
        • (+3h) = time

        (Hope I am not misunderstanding what you meant)

         
  • Jeff Jensen

    Jeff Jensen - 2019-03-16
    • Status: open --> rejected
     

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.