Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-293

data import - nulling fields impossible

    Details

    • Sprint:
      Bug Crush Event - 21/2/2015, Community Day 3 - 2015

      Description

      It's a copy of the old-jira issue http://jira.undersunconsulting.com/browse/OFBIZ-462 Tarlika Elisabeth Schmitz

      =================================================
      I am on rev 5274.
      data import from .xml file: empty attributes (e.g. description="") are ignored and the field retains its former value.

      Comment by Chris Juettner [04/Oct/05 01:51 PM]
      This patch comments out the check for null or empty values in the data XML file found by the EntitySaxReader. I also added an additional warning message in case you do not want to persist empty values to the database but still would like to know what happened.

      Comment by Si Chen [04/Oct/05 01:59 PM]
      Chris-
      Just a comment: it's very important that a field in the entity engine is set to null ONLY when the XML file specifically has an empty attribute (ie, description=""). Otherwise, there's a lot of seed data that is in separate XML files, and they could overwrite each other.

      Comment by Chris Juettner [10/Oct/05 11:56 AM] [ Permlink ]
      Si,
      I'm not sure I understand your comment about seed data in seperate XML files overwriting each other. Shouldn't seed data XML files be entity specific? Why would one seed data XML file overwrite another?

      Do you have any comment on what a better solution is for seed data values that could be empty?
      Thanks
      Chris

      1. ofbiz-462-patch.txt
        2 kB
        Marco Risaliti
      2. ofbiz-293-patch.txt
        1 kB
        Tim Lo
      3. OFBIZ-293_set-empty-strings-as-null.patch
        1 kB
        Martin Becker

        Issue Links

          Activity

          Hide
          risalitm Marco Risaliti added a comment -

          Someone knows if is this still an issue or can I close it ?
          I'm the reporter only because I have copy it from the old jira issue.

          Show
          risalitm Marco Risaliti added a comment - Someone knows if is this still an issue or can I close it ? I'm the reporter only because I have copy it from the old jira issue.
          Hide
          jacopoc Jacopo Cappellato added a comment -

          Marco,

          this is still an open issue... I'd suggest to keep it open.

          Show
          jacopoc Jacopo Cappellato added a comment - Marco, this is still an open issue... I'd suggest to keep it open.
          Hide
          adrianc@hlmksw.com Adrian Crum added a comment -

          What if we used something like description="$

          {null}

          " and modified the reader to look for that string and treat it as a null value?

          Show
          adrianc@hlmksw.com Adrian Crum added a comment - What if we used something like description="$ {null} " and modified the reader to look for that string and treat it as a null value?
          Hide
          hansbak Hans Bakker added a comment -

          isn't it so that in minilanguage we can use the environment variable "nullField"? isn't the same true here?

          Show
          hansbak Hans Bakker added a comment - isn't it so that in minilanguage we can use the environment variable "nullField"? isn't the same true here?
          Hide
          tlofolica Tim Lo added a comment -

          Updated the patch as per Si's comment.

          Show
          tlofolica Tim Lo added a comment - Updated the patch as per Si's comment.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          revision 8458 ?

          Show
          jacques.le.roux Jacques Le Roux added a comment - revision 8458 ?
          Hide
          mbecker Martin Becker added a comment -

          I've attached an updated version of the patch from 2011.

          In my opinion it would be straight forward, if the entity import would treat fields that are mentioned in entity XML files as fields that should be updated always, as the GenericDAO does. The current logic ignores fields, if there is only an empty string as value.

          The enhancement brought by the solution of OFBIZ-4949 makes it possible to generally configure an entity-engine-xml file to be interpreted as replacement, so that all NOT mentioned fields are set to NULL instead of leaving the old value while updating the entity.

          Nevertheless I think, that the default (update) behavior, the one with fine grained control over which fields are touched, should also give the possibility to set field values to NULL by including them in the XML.

          The solution with the lowest impact on existing XML files and the most natural meaning should be to interpret empty string values in the XML as NULL, as it is today. The only difference would be, that they are not longer ignored but set as NULL values in the database. The only drawback I see is, that you're still not able to set "empty string" values as concrete values to the database (no loss of function, because you can't do it today, too).

          As far as I can see, there should be no conflict with current existing data files. There are some where for example a parentTypeId is contained with empty string as value. Interpreting it as NULL and setting in database is exactly what is meant by such a definition.

          Show
          mbecker Martin Becker added a comment - I've attached an updated version of the patch from 2011. In my opinion it would be straight forward, if the entity import would treat fields that are mentioned in entity XML files as fields that should be updated always, as the GenericDAO does. The current logic ignores fields, if there is only an empty string as value. The enhancement brought by the solution of OFBIZ-4949 makes it possible to generally configure an entity-engine-xml file to be interpreted as replacement, so that all NOT mentioned fields are set to NULL instead of leaving the old value while updating the entity. Nevertheless I think, that the default (update) behavior, the one with fine grained control over which fields are touched, should also give the possibility to set field values to NULL by including them in the XML. The solution with the lowest impact on existing XML files and the most natural meaning should be to interpret empty string values in the XML as NULL, as it is today. The only difference would be, that they are not longer ignored but set as NULL values in the database. The only drawback I see is, that you're still not able to set "empty string" values as concrete values to the database (no loss of function, because you can't do it today, too). As far as I can see, there should be no conflict with current existing data files. There are some where for example a parentTypeId is contained with empty string as value. Interpreting it as NULL and setting in database is exactly what is meant by such a definition.
          Hide
          soledad Nicolas Malin added a comment -

          Maybe an other idea to manage null field, would be authorize FlexibleStringExpender and prepare the context with standard variable. The result will be as example :

          <Product productId="MyProduct" brandName="${nullField}"/>
          
          Show
          soledad Nicolas Malin added a comment - Maybe an other idea to manage null field, would be authorize FlexibleStringExpender and prepare the context with standard variable. The result will be as example : <Product productId= "MyProduct" brandName= "${nullField}" />
          Hide
          paul_foxworthy Paul Foxworthy added a comment - - edited

          Please see my discussion in OFBIZ-4602. The canonical XML way to specify a null value is to use the xsi:nil attribute (references to XML standards in OFBIZ-4602). The content of an element should be the data, and no matter what special value you attempt to reserve (an empty string, the word "null" enclosed in square, curly or angle brackets, etc) something will misinterpret that.

          Show
          paul_foxworthy Paul Foxworthy added a comment - - edited Please see my discussion in OFBIZ-4602 . The canonical XML way to specify a null value is to use the xsi:nil attribute (references to XML standards in OFBIZ-4602 ). The content of an element should be the data , and no matter what special value you attempt to reserve (an empty string, the word "null" enclosed in square, curly or angle brackets, etc) something will misinterpret that.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          I had assigned myself but unassigned, the reason is my comment I then put in OFBIZ-1602. For now at least, I have other priorities...

          Show
          jacques.le.roux Jacques Le Roux added a comment - I had assigned myself but unassigned, the reason is my comment I then put in OFBIZ-1602 . For now at least, I have other priorities...
          Hide
          mbecker Martin Becker added a comment -

          The use of the xsi:nil attribute is basically correct but is no solution for the problem we have to represent NULL values in XML element attributes, used for entity attributes. As I see, the xsi:nil attribute is only for defining a XML element as NULL, in our use case the entity itself.

          My thought was to improve the current logic without compromising current legacy data and usage. For now, empty string attribute values are ignored when loading XML entity data, that is just not correct. If we treat them as empty string, what would be the most correct solution, we not really gain a functional benefit but need to fix existing entity data files with for example ...parentTypeId=""... in it.

          For a full update while syncing data over the entity xml mechanism, the current solution with an additional specified action like "REPLACE" meets the requirements and the "XML Standard" by interpreting missing entity field (xml attributes) as NULL values.

          The gap I want to fill is the default action for loading entity XML data, where only explicitly defined fields are part of an update and there should be the possibility to "clear" single field values. There should only be very rare cases where someone would want to define and insert empty strings instead of NULL. Not even the current logic support this.

          Show
          mbecker Martin Becker added a comment - The use of the xsi:nil attribute is basically correct but is no solution for the problem we have to represent NULL values in XML element attributes, used for entity attributes. As I see, the xsi:nil attribute is only for defining a XML element as NULL, in our use case the entity itself. My thought was to improve the current logic without compromising current legacy data and usage. For now, empty string attribute values are ignored when loading XML entity data, that is just not correct. If we treat them as empty string, what would be the most correct solution, we not really gain a functional benefit but need to fix existing entity data files with for example ...parentTypeId=""... in it. For a full update while syncing data over the entity xml mechanism, the current solution with an additional specified action like "REPLACE" meets the requirements and the "XML Standard" by interpreting missing entity field (xml attributes) as NULL values. The gap I want to fill is the default action for loading entity XML data, where only explicitly defined fields are part of an update and there should be the possibility to "clear" single field values. There should only be very rare cases where someone would want to define and insert empty strings instead of NULL. Not even the current logic support this.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          I carefully reviewed propositions/comments by Adrian, Nicolas and Paul above (with corresponding code review) and I must say I agree with Martin. His proposition is simple, neat and fits in the current implementation of handling null fields while importing (see r1425813). So I will commit it soon, except if someone really identifies an issue or propose a better (and still simple ) solution.

          I though still regret that we have this dichotomy with null and null-field (nullField) between Entities and Minilang. It makes things confusing and it would be best if we would have used only one. I guess null, but in my OFBIZ-1602 comment above I wrote that nullField was introduced because of a Freemarker issue with null (I did not research again, I guess I was right then).
          At least in Minilang having only one form would definitively clarify this aspect. I remember having tried to use "null" instead of "nullField" in a condition-expr and not succeeded when nullField worked. I though can OOTB find condition-exprS using "null" which now makes me doubt :/. And also why not using nullField in if-compare. Also we have this difference in screen/form action? OK, too much for tonight but I feel we are almost there...

          Show
          jacques.le.roux Jacques Le Roux added a comment - I carefully reviewed propositions/comments by Adrian, Nicolas and Paul above (with corresponding code review) and I must say I agree with Martin. His proposition is simple, neat and fits in the current implementation of handling null fields while importing (see r1425813). So I will commit it soon, except if someone really identifies an issue or propose a better (and still simple ) solution. I though still regret that we have this dichotomy with null and null-field (nullField) between Entities and Minilang. It makes things confusing and it would be best if we would have used only one. I guess null, but in my OFBIZ-1602 comment above I wrote that nullField was introduced because of a Freemarker issue with null (I did not research again, I guess I was right then). At least in Minilang having only one form would definitively clarify this aspect. I remember having tried to use "null" instead of "nullField" in a condition-expr and not succeeded when nullField worked. I though can OOTB find condition-exprS using "null" which now makes me doubt :/. And also why not using nullField in if-compare. Also we have this difference in screen/form action? OK, too much for tonight but I feel we are almost there...
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Clarifying these points would also help to fix OFBIZ-4602

          Show
          jacques.le.roux Jacques Le Roux added a comment - Clarifying these points would also help to fix OFBIZ-4602
          Hide
          soledad Nicolas Malin added a comment -

          +1, my proposition is an other improvement

          the Martin's Solution is easier.

          Show
          soledad Nicolas Malin added a comment - +1, my proposition is an other improvement the Martin's Solution is easier.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks for feedback Nicolas

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks for feedback Nicolas
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks Martin

          Your patch is in
          trunk r1706316
          R14.12 r1706317
          R13.07 r1706318
          R12.04 r1706319

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks Martin Your patch is in trunk r1706316 R14.12 r1706317 R13.07 r1706318 R12.04 r1706319

            People

            • Assignee:
              jacques.le.roux Jacques Le Roux
              Reporter:
              risalitm Marco Risaliti
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development

                  Agile