FWD » Core Development » Database

In DMO classes, getter and setter names are created automatically by conversion, based on the field names which are application-specific, so they can be anything matching getter or setter creation rules.

There is possibility that these automatically created names may shadow method names in FWD runtime library, which will lead to runtime errors, which is hard to identify. The example is #6694 issue.

In this task, all such possible name conflicts needs to be identified, and methods, defined in FWD runtime, renamed to names, which are do not match the getter/setter pattern, making the name conflict impossible for any converted customer code.

To clarify the task, we need to work through all superinterfaces of the persist.Buffer interface, along with all the ancestors of those superinterfaces, to make sure none of the method names defined in that hierarchy of interfaces start with get, is, or set.

The typical convention we use to prevent this conflict is to use just the base name of the property represented by the method as the method name. For example, consider an interface named Foo, which allows getting and setting properties named bar and baz. Assume bar is a boolean property and baz is a String property. Today, the interface Foo likely defines these methods:

public boolean isBar();
public void setBar(boolean bar);
public String getBaz();
public void setBaz(String baz);

These would be changed to:

public boolean bar();
public void bar(boolean bar);
public String baz();
public void baz(String baz);

Any such changes need to be worked back through 4GL method conversion rules, to make sure the changes are reflected there as well.

Eric Faulhaber wrote:

To clarify the task, we need to work through all superinterfaces of the persist.Buffer interface, along with all the ancestors of those superinterfaces, to make sure none of the method names defined in that hierarchy of interfaces start with get, is, or set.

Are we considering getters/setters only, or any methods with matching names? For example, need we fix methods like this: character getCallbackProcName(String)?

Here is the list of matching by name methods:

character getADMData()
character getCallbackProcName(String)
character getCallbackProcName(Text)
character getDbName()
character getSerializeName()
character getTable()
handle getCallbackProcContext(String)
handle getCallbackProcContext(Text)
handle getQueryAsHandle()
integer getUniqueID()
logical getChanges(handle)
logical getChanges(handle,boolean)
logical getChanges(handle,logical)
logical isDataSourceModified()
logical isMultiTenant()
logical setCallback(String,String)
logical setCallback(String,String,handle)
logical setCallback(String,String,object)
logical setCallback(String,Text,handle)
logical setCallbackProcedure(character,character)
logical setCallbackProcedure(character,character,handle)
logical setCallbackProcedure(character,String)
logical setCallbackProcedure(character,String,handle)
logical setCallbackProcedure(String,character)
logical setCallbackProcedure(String,character,handle)
logical setCallbackProcedure(String,String)
logical setCallbackProcedure(String,String,handle)
logical setCallback(Text,Text)
logical setCallback(Text,Text,handle)
logical setCallback(Text,Text,object)
static logical isDataSourceModified(Buffer)
void setADMData(character)
void setADMData(String)
void setCurrentIteration(handle)
void setDataSourceModified(boolean)
void setDataSourceModified(logical)
void setMultiTenant(boolean)
void setMultiTenant(logical)
void setQueryAsHandle(handle)
void setQueryAsHandle(QueryWrapper)
void setSerializeName(String)
void setSerializeName(Text)

the true getters/setters are:

character getADMData()
character getDbName()
character getSerializeName()
character getTable()
handle getQueryAsHandle()
integer getUniqueID()
logical isDataSourceModified()
logical isMultiTenant()
static logical isDataSourceModified(Buffer)
void setADMData(character)
void setADMData(String)
void setCurrentIteration(handle)
void setDataSourceModified(boolean)
void setDataSourceModified(logical)
void setMultiTenant(boolean)
void setMultiTenant(logical)
void setQueryAsHandle(handle)
void setQueryAsHandle(QueryWrapper)
void setSerializeName(String)
void setSerializeName(Text)

Vladimir Tsichevski wrote:

Eric Faulhaber wrote:

To clarify the task, we need to work through all superinterfaces of the persist.Buffer interface, along with all the ancestors of those superinterfaces, to make sure none of the method names defined in that hierarchy of interfaces start with get, is, or set.

Are we considering getters/setters only, or any methods with matching names? For example, need we fix methods like this: character getCallbackProcName(String)?

Sorry, I missed this question earlier.

DMO getters/setters can be of the form:

// scalar field methods
<BDT>   get<Name>()
logical is<Name>()
void    set<Name>(<BDT>)

// extent field methods
<BDT>   get<Name>(int)
<BDT>   get<Name>(NumberType)
logical is<Name>(int)
logical is<Name>(NumberType)
void    set<Name>(int, <BDT>)
void    set<Name>(NumberType, <BDT>)

// special bulk access extent field methods
<BDT>[] get<Name>()
void    set<Name>(<BDT>)
void    set<Name>(<BDT>[])

Where <BDT> represents a BaseDataType subclass.

Any signatures which can conflict with these forms should be replaced.

Eric Faulhaber wrote:

DMO getters/setters can be of the form:

[...]

So, the following methods from the list in #6767-5 do match the criteria, and must be renamed:

character getADMData()
character getDbName()
character getSerializeName()
character getTable()
handle getQueryAsHandle()
integer getUniqueID()
logical isDataSourceModified()
logical isMultiTenant()
void setADMData(character)
void setCurrentIteration(handle)
void setDataSourceModified(logical)
void setMultiTenant(logical)
void setQueryAsHandle(handle)
void setSerializeName(Text)

and these do not match and should be left intact:

character getCallbackProcName(String)
character getCallbackProcName(Text)
handle getCallbackProcContext(String)
handle getCallbackProcContext(Text)
logical getChanges(handle)
logical getChanges(handle,boolean)
logical getChanges(handle,logical)
static logical isDataSourceModified(Buffer)
logical setCallback(String,String)
logical setCallback(String,String,handle)
logical setCallback(String,String,object)
logical setCallback(String,Text,handle)
logical setCallbackProcedure(character,character)
logical setCallbackProcedure(character,character,handle)
logical setCallbackProcedure(character,String)
logical setCallbackProcedure(character,String,handle)
logical setCallbackProcedure(String,character)
logical setCallbackProcedure(String,character,handle)
logical setCallbackProcedure(String,String)
logical setCallbackProcedure(String,String,handle)
logical setCallback(Text,Text)
logical setCallback(Text,Text,handle)
logical setCallback(Text,Text,object)
void setADMData(String)
void setDataSourceModified(boolean)
void setMultiTenant(boolean)
void setQueryAsHandle(QueryWrapper)
void setSerializeName(String)

Some more changes needed, since some TRPL properties do not exist anymore, and the method call must be used now instead of property name:

=== modified file 'rules/annotations/record_scoping.rules'
--- rules/annotations/record_scoping.rules    2022-06-27 00:16:13 +0000
+++ rules/annotations/record_scoping.rules    2022-10-11 11:40:27 +0000
@@ -455,7 +455,7 @@
                <action>scope.putAnnotation("is_dynamic_table", true)</action>
             </rule>
             <action>scope.putAnnotation("schemaname", master.schemaName)</action>
-            <action>scope.putAnnotation("dbname", master.dbName)</action>
+            <action>scope.putAnnotation("dbname", master.dbName())</action>
             <action>scope.putAnnotation("scopetype", #(long) bufscope.type)</action>
             <action>scope.putAnnotation("implicit", master.implicit and !master.inSuper)</action>
             <action>scope.putAnnotation("readOnly", master.readOnly)</action>

=== modified file 'rules/convert/builtin_functions.rules'
--- rules/convert/builtin_functions.rules    2022-10-05 08:32:33 +0000
+++ rules/convert/builtin_functions.rules    2022-10-11 11:44:45 +0000
@@ -578,7 +579,7 @@

          <!-- DATA-SOURCE-MODIFIED function -->
          <rule>ftype == prog.kw_data_sm
-            <action>methodText = "DataSourceModifiable.isDataSourceModified"</action>
+            <action>methodText = "DataSourceModifiable.dataSourceModified()"</action>
             <action>dbimport = true</action>
          </rule>

Please post the final proposed patch.

Eric: Please review.

Eric: This is still waiting for review.

Code review 6767-full.diff:

The changes look good.

Just some minor issues with the file headers:

• The dates are set to "2022yyyy". Regardless of the date and order of when these changes get committed, the date should reflect when the work was done.
• Older entries can be changed for format or to correct typos, but the content should not be altered to reflect later changes to the code. For example, in BufferImpl.java, the 20131128 entry should not be updated to reflect the new name of the uniqueID method. It was named getUniqueID at that time and should be left that way, since the entries are meant to be historical.

Eric Faulhaber wrote:

Code review 6767-full.diff:

The changes look good.

Just some minor issues with the file headers:

• The dates are set to "2022yyyy". Regardless of the date and order of when these changes get committed, the date should reflect when the work was done.

Got it. I will put today's date in this very case. From now on will be follow your rules.

• Older entries can be changed for format or to correct typos, but the content should not be altered to reflect later changes to the code. For example, in BufferImpl.java, the 20131128 entry should not be updated to reflect the new name of the uniqueID method. It was named getUniqueID at that time and should be left that way, since the entries are meant to be historical.

That is understandable. History can only be rewritten in Orwell's novels or here in Russia (where I fill myself living in the "1984" :-( )

What am I expected to do to make this patch merged to the trunk?

Vladimir Tsichevski wrote:

What am I expected to do to make this patch merged to the trunk?

Create a task branch 6767a, apply the diffs, and we'll follow the usual procedure. This has lower priority than #3827.

Eric Faulhaber wrote:

Vladimir Tsichevski wrote:

What am I expected to do to make this patch merged to the trunk?

Create a task branch 6767a, apply the diffs, and we'll follow the usual procedure. This has lower priority than #3827.

Done. Revno is 14522. Also some minor history issues cleaned up. Please, review.

Eric: Please review.

Vladimir, sorry for the long delay in reviewing this again. Please rebase this branch when you can. I started reviewing 6767a/14522, but I suspect some of the changes may already be in trunk in some form, due to bugs having been found/fixed in the time since this branch was created.

Eric Faulhaber wrote:

Vladimir, sorry for the long delay in reviewing this again. Please rebase this branch when you can. I started reviewing 6767a/14522, but I suspect some of the changes may already be in trunk in some form, due to bugs having been found/fixed in the time since this branch was created.

The branch is ~370 releases old, rebasing it causes many conflicts. I think, it is simpler to redo the task on the latest FWD release now than resolving all the conflicts manually :-(

The following methods should be also renamed in addition to the methods listed in #6767-9:

handle getIteration(int)
handle getIteration(NumberType)
character getXmlDataType()
setXmlDataType(Text)
character getXmlNodeType()
setXmlNodeType(Text)
character getXmlNodeName()
setXmlNodeName(Text)

The complete method list is:

public abstract com.goldencode.p2j.util.logical com.goldencode.p2j.persist.Buffer.isMultiTenant()
public abstract void com.goldencode.p2j.persist.Buffer.setMultiTenant(com.goldencode.p2j.util.logical)
public abstract com.goldencode.p2j.util.character com.goldencode.p2j.util.ADMData.getADMData()
public abstract void com.goldencode.p2j.util.ADMData.setADMData(com.goldencode.p2j.util.character)
public abstract com.goldencode.p2j.util.character com.goldencode.p2j.util.DatabaseInfo.getTable()
public abstract com.goldencode.p2j.util.character com.goldencode.p2j.util.DatabaseInfo.getDbName()
public abstract com.goldencode.p2j.util.logical com.goldencode.p2j.persist.DataSourceModifiable.isDataSourceModified()
public abstract void com.goldencode.p2j.persist.DataSourceModifiable.setDataSourceModified(com.goldencode.p2j.util.logical)
public abstract void com.goldencode.p2j.util.IterableResource.setCurrentIteration(com.goldencode.p2j.util.handle)
public abstract com.goldencode.p2j.util.handle com.goldencode.p2j.util.IterableResource.getIteration(int)
public default com.goldencode.p2j.util.handle com.goldencode.p2j.util.IterableResource.getIteration(com.goldencode.p2j.util.NumberType)
public abstract com.goldencode.p2j.util.character com.goldencode.p2j.persist.NamedSerializable.getSerializeName()
public abstract void com.goldencode.p2j.persist.NamedSerializable.setSerializeName(com.goldencode.p2j.util.Text)
public abstract void com.goldencode.p2j.persist.QueryAssociable.setQueryAsHandle(com.goldencode.p2j.util.handle)
public abstract com.goldencode.p2j.util.handle com.goldencode.p2j.persist.QueryAssociable.getQueryAsHandle()
public abstract com.goldencode.p2j.util.integer com.goldencode.p2j.util.UniqueID.getUniqueID()
public abstract com.goldencode.p2j.util.character com.goldencode.p2j.persist.XmlNode.getXmlDataType()
public abstract void com.goldencode.p2j.persist.XmlNode.setXmlDataType(com.goldencode.p2j.util.Text)
public abstract com.goldencode.p2j.util.character com.goldencode.p2j.persist.XmlNode.getXmlNodeType()
public abstract void com.goldencode.p2j.persist.XmlNode.setXmlNodeType(com.goldencode.p2j.util.Text)
public abstract com.goldencode.p2j.util.character com.goldencode.p2j.persist.XmlNode.getXmlNodeName()
public abstract void com.goldencode.p2j.persist.XmlNode.setXmlNodeName(com.goldencode.p2j.util.Text)

The work redone in the branch 6767b, The branch is rebased to trunk rev. 14906 the branch revision is 14917.
Please, review the cumulative change 14906-14917.

Vladimir Tsichevski wrote:

The work redone in the branch 6767b, The branch is rebased to trunk rev. 14906 the branch revision is 14917.
Please, review the cumulative change 14906-14917.

Review, please. I will need another heavy rebase already to jump over 11-month long gap :-(

Vladimir, did you use some script to 'sed' the changes? As Eric mentioned, there are lots of unintended changes. The LegacyJavaAppserver, LegacyJavaAppserverAPI, DataSetContainer, DataSetParameter, and many others have changes which need to be backed out - those methods defs are not inherited from the problematic Buffer/HandleChain interfaces.

Only changes which need to remain must come from renaming the Java methods from the Buffer interface and its super-interfaces and HandleChain (as the DMO proxy is built from BufferImpl.java, not Buffer interface). And this refactor can be done automatically (with CTRL+SHIFT+G) in Eclipse, although each one needs to be done 'by hand'. Otherwise, there are 'static proxy' resources like SESSION (via CommonSession interface, proxied by SessionUtils and other classes), which need to be checked 'by hand'.

Again, this needs to be done by hand, a sed script or 'search and replace' is too destructive.

And also, the branch is too far behind trunk, it needs to be rebased before is reworked (or we move to a different branch).

Constantin Asofiei wrote:

Vladimir, did you use some script to 'sed' the changes?

No. I used Java reflection to collect the methods matching the requirements. Then I did the method renamings in Eclipse.

As Eric mentioned, there are lots of unintended changes. The LegacyJavaAppserver, LegacyJavaAppserverAPI, DataSetContainer, DataSetParameter, and many others have changes which need to be backed out - those methods defs are not inherited from the problematic Buffer/HandleChain interfaces.

Will see into it.

Only changes which need to remain must come from renaming the Java methods from the Buffer interface and its super-interfaces and HandleChain (as the DMO proxy is built from BufferImpl.java, not Buffer interface).

If I rememnber correctly, I did not process the HandleChain. Will check this.

And this refactor can be done automatically (with CTRL+SHIFT+G) in Eclipse, although each one needs to be done 'by hand'.

I used exactly this method.

Otherwise, there are 'static proxy' resources like SESSION (via CommonSession interface, proxied by SessionUtils and other classes), which need to be checked 'by hand'.

Will re-check.

Again, this needs to be done by hand, a sed script or 'search and replace' is too destructive.

I would say, sed is rather useless for the purpose.

And also, the branch is too far behind trunk, it needs to be rebased before is reworked (or we move to a different branch).

I’ve done this once a year while this task awaited review. Based on my experience, it’s easier to redo the task on the most recent version than to rebase a year’s worth of changes :-( I created the 6767c to do this.

And this refactor can be done automatically (with CTRL+SHIFT+G) in Eclipse, although each one needs to be done 'by hand'.

Here I meant 'ALT-SHIFT-R' (CTRL-SHIFT-G is for 'find'), or right-click on a method/Refactor/Rename. The point is: find the method in the interface, and after that rename it, which will also rename all occurrences automatically. What confused me is how LegacyJavaAppserver ended up having methods renamed, this is almost de-coupled from the main FWD runtime (especially p2j.persist package).

Vladimir Tsichevski wrote:

I’ve done this once a year while this task awaited review. Based on my experience, it’s easier to redo the task on the most recent version than to rebase a year’s worth of changes :-( I created the 6767c to do this.

Thank you.

Eric, Constantin,
I need you to confirm this:

DMO setters do not use Java built-in types. For example, we can create a setter method like void setSerializeName(Text), but never void setSerializeName(String). However, because our conversion procedures share the same method name across different parameter types, we need to rename both setter methods.

For HandleChain and its ancestors I found 8 matching methods:

   character getPrivateData()
   handle getNextSibling()
   handle getPrevSibling()
   void setNextSibling(handle nextSibling)
   void setPrevSibling(handle prevSibling)
   void setPrivateData(BaseDataType data)
   void setPrivateData(character data)
   void setPrivateData(String data)

For Buffer and its ancestors I found 29 matching methods::

   character getADMData()
   character getDbName()
   character getSerializeName()
   character getTable()
   character getXmlDataType()
   character getXmlNodeName()
   character getXmlNodeType()
   handle getIteration(int level)
   handle getIteration(NumberType level)
   handle getQueryAsHandle()
   integer getUniqueID()
   logical isDataSourceModified()
   logical isMultiTenant()
   void setADMData(character value)
   void setADMData(String value)
   void setCurrentIteration(handle newIter)
   void setDataSourceModified(boolean mod)
   void setDataSourceModified(logical mod)
   void setMultiTenant(boolean l)
   void setMultiTenant(logical l)
   void setQueryAsHandle(handle qry)
   void setSerializeName(String sName)
   void setSerializeName(Text sName)
   void setXmlDataType(String newType)
   void setXmlDataType(Text newType)
   void setXmlNodeName(String name)
   void setXmlNodeName(Text name)
   void setXmlNodeType(String newType)
   void setXmlNodeType(Text newType)

Eric Faulhaber wrote:

Vladimir, this is a large set of changes. What testing do you propose?

I think, converting and compiling any customer application will reveal possible problems.

• AbstractTempTable
  • line 2938 - please add back defBuffer.bufferManager instead of BufferManager. The point is to avoid the context-local lookup.
• DynamiConversionHelper, FilteredResults, RandomAccessQuery, RecordBuffer.popTempContext
  • buildDataModelClass - PersistenceException was removed
• RecordBuffer
  • line 1277, please change to 20230505
• JsonExport, XmlExport
  • IOException and other exceptions were removed
• GenericFrame
  • line 749, please fix it
• HandleOps, SaxEntityImpl, XEntityImpl
  • missing history entry

Beside the removed exceptions, I think is OK. We need to convert fully customer applications and check for regressions, and also runtime.

Constantin Asofiei wrote:

Review of 6767c rev 15980.

• AbstractTempTable
  • line 2938 - please add back defBuffer.bufferManager instead of BufferManager. The point is to avoid the context-local lookup.

What do you mean? Elaborate, please. The isActiveBuffer method belongs to a final BufferManager class and does not refer any instance fields, so it was reasonable to make it static and refer it as static.

• DynamiConversionHelper, FilteredResults, RandomAccessQuery, RecordBuffer.popTempContext
  • buildDataModelClass - PersistenceException was removed

It was removed because it is never thrown.

• RecordBuffer
  • line 1277, please change to 20230505

Done.

• JsonExport, XmlExport
  • IOException and other exceptions were removed

They were removed because they are never thrown.

• GenericFrame
  • line 749, please fix it

My fault, fixed
Also fixed a similar error in XmlNode.

• HandleOps, SaxEntityImpl, XEntityImpl
  • missing history entry

Fixed

I have to fix my code, which manages history entries :-(

History entry problems fixed in 6767c rev. 15981.
I did not revert other changes you noted until they are proved wrong.

Beside the removed exceptions, I think is OK. We need to convert fully customer applications and check for regressions, and also runtime.

I think, to convert and compile will be enough. I do not see how these change could affect the runtime behavior.

Vladimir Tsichevski wrote:

Constantin Asofiei wrote:

Review of 6767c rev 15980.

• AbstractTempTable
  • line 2938 - please add back defBuffer.bufferManager instead of BufferManager. The point is to avoid the context-local lookup.

What do you mean? Elaborate, please. The isActiveBuffer method belongs to a final BufferManager class and does not refer any instance fields, so it was reasonable to make it static and refer it as static.

I see now, thanks, you are correct.

• DynamiConversionHelper, FilteredResults, RandomAccessQuery, RecordBuffer.popTempContext
  • buildDataModelClass - PersistenceException was removed

It was removed because it is never thrown.

I recalled wrong that PersistenceException is unchecked, but actually is a checked exception. And you are right, is never thrown. Thanks.

I did not revert other changes you noted until they are proved wrong.

Looks like you don't need to revert.

Constantin Asofiei wrote:

Looks like you don't need to revert.

Good, now I am running a customer big application conversion with 6767c...

Vladimir Tsichevski wrote:

Constantin Asofiei wrote:

Looks like you don't need to revert.

Good, now I am running a customer big application conversion with 6767c...

The conversion completed without issues, but the resulting code cannot be compiled due to multiple method-not-found errors.

We need to update the conversion rules, as the correct method names are now privateData - not getPrivateData or setPrivateData. Since the PRIVATE-DATA attribute is widely used across OE, we should use the privateData name in the conversion process and rename all matching methods in the FWD runtime.

Vladimir Tsichevski wrote:

We need to update the conversion rules, as the correct method names are now privateData - not getPrivateData or setPrivateData. Since the PRIVATE-DATA attribute is widely used across OE, we should use the privateData name in the conversion process and rename all matching methods in the FWD runtime.

Please double check that all attributes/methods from #6767-39 are fixed in methods_attributes.rules - sorry, I forgot to do this.

Otherwise, the other unknown is customer projects which use hand-written 4GL-compatible Java code; that code will need to be updated, too.

Constantin Asofiei wrote:

Vladimir Tsichevski wrote:

We need to update the conversion rules, as the correct method names are now privateData - not getPrivateData or setPrivateData. Since the PRIVATE-DATA attribute is widely used across OE, we should use the privateData name in the conversion process and rename all matching methods in the FWD runtime.

Please double check that all attributes/methods from #6767-39 are fixed in methods_attributes.rules - sorry, I forgot to do this.

I’ve already done the check. The only exception is the PRIVATE-DATA accessors, which present a problem due to naming conflicts, as this attribute is also used across TreeFace and its class subtree. We should distinguish these cases within the conversion rules, perhaps using conditional logic.

Otherwise, the other unknown is customer projects which use hand-written 4GL-compatible Java code; that code will need to be updated, too.

We can address this later. Are there any known examples?

Vladimir Tsichevski wrote:

We should distinguish these cases within the conversion rules, perhaps using conditional logic.

No, we can not do this - a h:private-data is like a pointer, you don't know what the h handle holds. TreeFace and TreeWidgetBase have a separate problem, they should not have had PRIVATE-DATA added directly - these should have been separated in a standalone interface, which will be inherited by them and CommonHandleChain. Please don't change TreeFace and TreeWidgetBase - if get/setPrivateData is used, then is used from customer's hand-written implementation of these, not from conversion. Eugenie may now more (please don't post customer code here).

We can address this later. Are there any known examples?

I don't know, we will see when we reconvert apps.

I’m struggling to get the conversion to work as intended. For example, I’d like the NEXT-SIBLING attribute converted to nextSibling. To achieve this, I modified the methodText in the following lines of the method_attributes.rules file:

               <rule>ftype == prog.kw_next_sib
                  <action>hwrap = ""</action>
                  <action>methodText = "getNextSibling"</action>
               </rule>

I updated methodText from getNextSibling to nextSibling. However, this change didn’t help, as the conversion still generates getNextSibling. What could be the issue?

Vladimir Tsichevski wrote:

I updated methodText from getNextSibling to nextSibling. However, this change didn’t help, as the conversion still generates getNextSibling. What could be the issue?

What test are you using to check this?

Constantin Asofiei wrote:

Vladimir Tsichevski wrote:

I updated methodText from getNextSibling to nextSibling. However, this change didn’t help, as the conversion still generates getNextSibling. What could be the issue?

What test are you using to check this?

// Test the conversion of NEXT-SIBLING, PREV-SIBLING, and PRIVATE-DATA attributes in branch 6767c.

DEFINE VARIABLE hwMenu AS HANDLE NO-UNDO.

hwMenu = hwMenu:NEXT-SIBLING.
hwMenu = hwMenu:PREV-SIBLING.
MESSAGE hwMenu:PRIVATE-DATA.

The conversion with 6767c gives me this:

         hwMenu.assign(hwMenu.unwrap().nextSibling());
         hwMenu.assign(hwMenu.unwrap().prevSibling());
         message(hwMenu.unwrap().getPrivateData());

The compile error is on getPrivateData.

Constantin Asofiei wrote:

The conversion with 6767c gives me this:
[...]

The compile error is on getPrivateData.

The issue was a false alarm due to mistakenly using the trunk for conversion. I fixed the PRIVATE-DATA conversion in revision 15982, ensuring NEXT-SIBLING, PREV-SIBLING, and PRIVATE-DATA attributes now convert to their new names correctly.

Tested branch 6767c with some examples, all of which convert, compile, and run successfully. Next, I will rebuild with 6767c and test our large customer application.

Tested the branch with our big customer application, and no issues were identified. We can also validate using our ChUI test suite; however, I require someone to perform this on my behalf, as the suite does not execute correctly on my workstation. Additionally, I see no further tasks at this time.

6767c rebased to the current trunk, rev now is 16018.

Vladimir, please look into TreeFace.java and TreeWidgetBase.java - these define i.e. get/setPrivateData, while the impl inherits also privateData() from BaseEntity.

Constantin Asofiei wrote:

Vladimir, please look into TreeFace.java and TreeWidgetBase.java - these define i.e. get/setPrivateData, while the impl inherits also privateData() from BaseEntity.

These methods (getPrivateDate(character) and setPrivateData(character, character)) do not align with any patterns outlined in #6767-8, thus posing no issues.

Vladimir Tsichevski wrote:

Constantin Asofiei wrote:

Vladimir, please look into TreeFace.java and TreeWidgetBase.java - these define i.e. get/setPrivateData, while the impl inherits also privateData() from BaseEntity.

These methods (getPrivateDate(character) and setPrivateData(character, character)) do not align with any patterns outlined in #6767-8, thus posing no issues.

For reasons no one can presently recall (from the early days when the world was young), the NodeTag property was mapped to PrivateData by Hynek in revision 11315.

Argh, this is OCX.

Hynek: can you point Vladimir to an example how TreeFace emits get/setPrivateData via OCX?

Constantin Asofiei wrote:

Argh, this is OCX.

It was, but it is not in FWD.

Hynek: can you point Vladimir to an example how TreeFace emits get/setPrivateData via OCX?

It would be nice.

If there is no reason to use PRIVATE-DATA in this context, switching to NodeTag could minimize confusion. Regardless, employing the get/setPrivateData methods does not introduce any issues.

The private data getter and setter was added by Vladimir in the context of #5144. I don't recall PRIVATE-DATA needed for OCX objects.

Hynek Cihlar wrote:

The private data getter and setter was added by Vladimir in the context of #5144. I don't recall PRIVATE-DATA needed for OCX objects.

I just added missing implementations of the methods matching this line in rules/annotations/ocx_conversion.rules:

         <action>map2.put("nodetag"               , "PrivateData")</action>

which was introduced in this release:

revno: 11315 [merge]
committer: Hynek Cihlar <hc@goldencode.com>
branch nick: trunk
timestamp: Tue 2019-06-11 20:00:00 +0200
message:
  TREELIST widget improvements - column move, sort, added new methods and attributes, improved conversion. Refs #3766.
  TABSET widget improvements - missing interface features and various fixes. Refs #3767.
  Implemented direct widget-level font control. Refs #3876.

So I am not the person to blame :-).

If nobody can recall why mysterious this line was introduced, we can replace it by:

         <action>map2.put("nodetag"               , "NodeTag")</action>

and rename the methods correspondingly.

Vladimir Tsichevski wrote:

So I am not the person to blame :-).

OK, you get only a partial blame :-).

I think the idea was to emit get/setPrivateData and let the conversion cast the object to CommonHandleChain. But yes, adding the support for NodeTag directly, as you already did since then, makes more sense.

If nobody can recall why mysterious this line was introduced, we can replace it by:

Yes.

Can this task be merged?

Hynek/Constantin/Eric: What testing is needed here?

Should this be in Internal Test status?

Greg Shah wrote:

Hynek/Constantin/Eric: What testing is needed here?

Should this be in Internal Test status?

Eric: Please assess this task ASAP.

Greg Shah wrote:

Greg Shah wrote:

Hynek/Constantin/Eric: What testing is needed here?

Should this be in Internal Test status?

Eric: Please assess this task ASAP.

Any news?