FWD » Core Development » Base Language

Priority features:

CREATE-X-NODEREF; CREATE-X-DOCUMENT; ATTRIBUTES: NAME, ATTRIBUTE-NAMES, OWNER-DOCUMENT, NUM-CHILDREN, UNIQUE-ID; METHODS: create-node(), append-child(), save(), delete-node(), insert-attribute(), create-node-namespace(), get-attribute(), set-attribute(), load(), get-document-element(), get-child(), remove-child();

I'm adding myself as watcher to be able to automatically receive the update notifications.

As with the other work for milestone 4, please only make the changes required for conversion (and compilation of the converted result).

Start by writing some basic samples that use all of these features. Then design and implement the conversion support.

It is OK to make this support more complete than listed above. In fact that is preferable, if it can be done without undue delay. We need you to finish this work by Friday.

INSERT-ATTRIBUTE is a documented method (in the 4GL reference). But it is for the SAX XML support, not for DOM. I mistakenly included it in the list for this task.

The implementation/design point to discuss. I'm looking for the place where we can implement the required DOM XML support.
1. The class com.goldencode.util.XmlHelper working with org.w3c.dom.Document data types and has some features we can use here, but not all we need is implemented there.
2. The package com.goldencode.p2j.directory is designed for other purpose if my understanding is correct. To handle P2J server tasks.
3. The class com.goldencode.p2j.util.handle can also be used to support Document handles.

So do we need to create separate Node related data type, probably extending current handle class? Or modify handle to include support for DOM XML objects? I would prefer to centralize the implementation logic into one or two separate class with usage the com.goldencode.util.XmlHelper class where it is required. The com.goldencode.p2j.util.handle class is good but X-document/noderef object handle can have more features than handle itself and will overload the handle class unnecessary.

1. The class com.goldencode.util.XmlHelper working with org.w3c.dom.Document data types and has some features we can use here, but not all we need is implemented there.

XmlHelper is just a tool that we may or may not use from the runtime code. It is definitely not the place for the 4GL-compatible API.

2. The package com.goldencode.p2j.directory is designed for other purpose if my understanding is correct. To handle P2J server tasks.

Yes, this is unrelated.

3. The class com.goldencode.p2j.util.handle can also be used to support Document handles.

Yes and no. The handle class is the Java equivalent of the 4GL HANDLE type (as used in DEF VAR my-handle AS HANDLE).

A HANDLE in 4GL can refer to many different resource types. So too can our Java handle class.

Your job is to implement 2 new resource types. Lets call them XDocument and XNodeRef. These will be Java interfaces that define the APIs needed to implement the attributes and methods of DOM XML processing.

You must create 2 implementation classes XDocumentImpl and XNodeRefImpl. These classes implement their respective interfaces AND also must implement WrappedResource.

You then must have a replacement for the CREATE-X-DOCUMENT (which instantiates a new instance of XDocumentImpl) and CREATE-X-NODEREF (which instantiates a new instance of XNodeRefImpl). These can be inside a class called XmlFactory.

Put all these new classes in a new package com.goldencode.p2j.xml.

Then you must add 2 "unwrap" methods to the handle class (unwrapXDocument() and unwrapXNodeRef()).

And the methods_attributes.rules must be modified to use the proper unwrap type (XDocument or XNodeRef) depending on the attribute or method being converted.

...
And the methods_attributes.rules must be modified to use the proper unwrap type (XDocument or XNodeRef) depending on the attribute or method being converted.

OK. We also have to implement CREATE-X-DOCUMENT and CREATE-X-NODEREF conversion. I have scanned the rulesets and I guess the language_statements.rules is reasonable place(more than other ruleset files according to the fact we have the 4GL statement in this case) to put the conversion logic. If you have objections let me know please.

Yes, you can put that in language_statements.rules.

The point to discuss.

I'm implementing the attributes/methods conversion. It is OK with the current approach to deal with attributes/methods specific to either XDocument or XNodeRef. The problematic case is when attribute or method is common for both data types. For example Because in Progress both are the kind of handle. So once we create the object - we loose the information of the handle type from the P2J ConversionDriver point of view. And additionally we will need to duplicate the same code for both classes.

So I'm offering to create one more class and interface pair, say XEntity and XEntityImpl(or some other name) to accumulate the all common code for both DOM Object we currently implementing. Is it OK?

Another question is the attribute NAME has already implemented for handle class. So the processing of this attribute is up to handle class itself. Correct?

Yes, make an XEntity interface and make both the XDocument and XNodeRef extend that superinterface. That way they will both implement the same features. The backing implementation objects can inherit the same base class if the implementation is the same.

Yes, the name attribute is not needed for the XML objects.

There are two methods unusual:
get-child(x-node-ref, index) and get-document-element(x-node-ref). The both of them returns boolean error code and put the required result into x-node-ref. Actually this means we have to pass the reference to the x-node-ref so when we modify x-node-ref inside the method body the value in the calling procedure will also be changed. If my understanding correct if passing variable is the class instance - Java will pass them as reference. Correct? In this case we have no problems because classes are passed to methods using reference to the object. If this is not correct - we need to implement something different because we can not use the return value of these methods due to the return value is reserved for logical error code.

Another question:
The converted DOM XML sample should have the import statement to be compiled:
import com.goldencode.p2j.xml.*;
Can you point me the place to modify for this inline to be included into converted Java code.

Yes, both get-child(x-node-ref, index) and get-document-element(x-node-ref) have expressions of type handle as the 1st parameter. In Java, the expressions will naturally emit and you will not need to do anything for that case. Just define the matching Java parameter as handle.

For an example of how to add the import, search on "createImport" in rules/convert/methods_attributes.rules.

For an example of how to add the import, search on "createImport" in rules/convert/methods_attributes.rules.

Thanks. So I have finished the conversion modifications to support following:

STATEMENTS:
CREATE-X-NODEREF - KW_X_NODE;
CREATE-X-DOCUMENT - KW_X_DOC;

ATTRIBUTES:
ATTRIBUTE-NAMES - KW_ATTR_NAM
OWNER-DOCUMENT - KW_OWN_DOC
NUM-CHILDREN - KW_NUM_CHLN
UNIQUE-ID - KW_UNIQ_ID

METHODS:
append-child() - KW_APPEND_C
create-node() - KW_CREAT_ND
create-node-namespace() - KW_CREAT_NN
delete-node() - KW_DEL_NODE
get-attribute() - KW_GET_ATTR
get-child() - KW_GET_CHLD
get-document-element() - KW_GET_D_E
load() - KW_LOAD
remove-child() - KW_REM_CHLD
save() - KW_SAVE
set-attribute() - KW_SET_ATTR

Now shifting to the Java implementation.

Feedback:

1. The versions of existing files (like methods_attributes.rules and language_statements.rules) need to be merged to the latest level.

2. Please add all methods/attributes in alphabetical order, not as a related group.

3. Do not put more than 1 history entry in for the same change. For example, XEntity has a 001 and 002 history entry, but the file will only be checked in once for all this work. Just combine all the entries so that there is 1 and only 1 entry.

4. Add your missing history entries where needed.

5. getUniqId() should be getUniqueId() since it reads better.

6. Progress does not work with DOM Elements directly. Everything is hidden inside a handle and is processed using 4GL data types. There is no 4GL data type for a DOM Element. For this reason, there is no way to operate on DOM Elements directly from the 4GL. Everything must go through the attributes/methods they they provide. Since using the DOM Element class in Java would require different code than the original 4GL (for example, we would have to provide null and empty string checks in many places and we would have to provide more code to translate between types and so forth). For this reason, we are going to completely hide the Java DOM XML classes. I don't want ANY of them leaking out into the converted code. XEntity.getNode() is an example of this "leaking". It returns an Element instance and it should return a handle to a node instead. I'm sorry I haven't gotten to look at your previous code drops, so this feedback is coming late. Please eliminate all leaking of the DOM objects into the converted code. The best way to do this: remove the import import org.w3c.dom.*; from all of the interfaces (XEntity, XDocument, XNodeRef) and then you will have to remove all of the leaks in order to compile.

I want to be clear here: I like that the *Impl classes directly use the Java DOM Element and related classes. That is good. I just do NOT want any of that to leak into the converted code (through the interfaces).

7. I think the runtime implementation is not correct. We don't need to keep a "parallel" tree of handles for every real DOM Element. I believe we can internally operate on the Java DOM tree and then just wrap the return by creating the proper XNodeRefImpl or XDocumentImpl instance like this:

return new handle(new XNodeRefImpl(myElementBeingReturned));

But remember: we don't need to do the runtime right now. Only do enough to allow you to design the conversion and write the stubs. Don't spend more time on the runtime code right now.

8. Please do not ever use character.getValue() to obtain the string directly. If you need to "unwrap" a character instance, first you must check if it is unknown. If so, then you must implement the proper unknown behavior like the 4GL. If not unknown, then you call character.toStringMessage() to safely get the String out. That method does some work that is bypassed when you call getValue(). getValue() should not be used at all.

9. You have some code formatting problems:

      if(loadMode.equalsIgnoreCase("file"))
         try
         {
            xdoc = XmlHelper.parse((String)source, validate.getValue(), null);
         }
         catch (Exception e)
         {
//            throw new IOException("Error reading XML document from file", e);
         }
      else if(loadMode.equalsIgnoreCase("memptr"))
        return new logical(false);
      else
        return new logical(false);

You are missing the {} in the if, else if and else:

      if(loadMode.equalsIgnoreCase("file"))
      {
         try
         {
            xdoc = XmlHelper.parse((String)source, validate.getValue(), null);
         }
         catch (Exception e)
         {
//            throw new IOException("Error reading XML document from file", e);
         }
      }
      else if(loadMode.equalsIgnoreCase("memptr"))
      {
        return new logical(false);
      }
      else
      {
        return new logical(false);
      }

I know that the else and else if cases are just 1 line, but it is not a good practice (the coding rules strongly suggest using the curly braces). The "if" is definitely breaking the coding rules.

There are cases of the spacing being wrong:

if(writeMode.equalsIgnoreCase("stream"))
if( xnode.appendChild(elChild) != null )

Please code them like this:

if (writeMode.equalsIgnoreCase("stream"))
if (xnode.appendChild(elChild) != null)

Please make the updates above and upload the candidate for review. I want to get this into testing.

The point to discuss:

7. I think the runtime implementation is not correct. We don't need to keep a "parallel" tree of handles for every real DOM Element. I believe we can internally operate on the Java DOM tree and then just wrap the return by creating the proper XNodeRefImpl or XDocumentImpl instance like this:

return new handle(new XNodeRefImpl(myElementBeingReturned));

But remember: we don't need to do the runtime right now. Only do enough to allow you to design the conversion and write the stubs. Don't spend more time on the runtime code right now.

I've tried the similar approach. And you are right, the Progress does not know about Elements, Nodes. It works only with handles. But Java does not know about handles. Without additional mapping we can get Element associated with handle. But how can we get the child handle? Using handle(Element) we calls getChildNodes() and get the Element with number N, not handle. And we need to convert known Element to the appropriate handle to return. This mapping is not implemented neither in Java nor in handle class as far as I know. That's why I introduced another HashMap.

Code feedback:

1. Don't we need a getNode() method to match the one in Progress? It should not be removed. It should just return a handle instead of an Element. That method is also missing from the methods_attributes.rules.

2. The code formatting is still not 100% correct. For example, see the "if" and "else if" blocks on lines 428, 485, 494 of XDocumentImpl.

3. We never return null to converted code. Progress has no such idea and the converted code won't have extra code added to avoid NullPointerExceptions. So on line 123 of XNodeRefImpl, this should be a return new character() instead of a return null.

4. The getUniqId name is wrong in methods_attributes.rules.

In regard to your question:

But Java does not know about handles. Without additional mapping we can get Element associated with handle. But how can we get the child handle? Using handle(Element) we calls getChildNodes() and get the Element with number N, not handle. And we need to convert known Element to the appropriate handle to return.

My initial idea is that we just construct a new handle(XDocument) or new handle (XNodeRef) when we need to return a node. My guess is that the Progress code really will not depend on the handle references being the same for the same "contained" node. So we may have 2 or more different handles that all refer (or contain) the same Element instance because the multiple XNodeRef instances store the same Element reference. As long as we handle comparisons (like equality) properly, the converted code will hopefully not care. We will have to do more testing to see if we can provide that it never matters. If I am correct, then we will have a much simpler approach. For example, for a getChild() we can use the normal Element traversal techniques of DOM and then just construct a new handle(new XNodeRef(Element)) and it will work.

But all of this is about the runtime anyway, we don't need to do any more in that regard right now.

Please review the Progress 4GL Reference and make a list of all other DOM XML features which are not listed above. If there are only a few, we will want to add them (quickly) to the update.

The remaining attributes/methods:
Attributes

CHILD-NUM
ENCODING
HANDLE
INSTANTIATING-PROCEDURE
LOCAL-NAME
NAMESPACE-PREFIX
NAMESPACE-URI
NODE-VALUE
NONAMESPACE-SCHEMA-LOCATION
PUBLIC-ID
SCHEMA-LOCATION
SCHEMA-PATH
SUBTYPE
SUPPRESS-NAMESPACE-PROCESSING
SYSTEM-ID
TYPE

Methods:

ADD-SCHEMA-LOCATION
CLONE-NODE
GET-ATTRIBUTE-NODE
GET-PARENT
IMPORT-NODE
INITIALIZE-DOCUMENT-TYPE
INSERT-BEFORE
LONGCHAR-TO-NODE-VALUE
MEMPTR-TO-NODE-VALUE
NODE-VALUE-TO-LONGCHAR
NODE-VALUE-TO-MEMPTR
NORMALIZE
REMOVE-ATTRIBUTE
REPLACE-CHILD
SET-ATTRIBUTE-NODE

As you can see the amount is more than 10. Do we need them to be added?

No, don't add the low priority attributes/methods right now.

I am OK with the code as uploaded. It will go into testing next. But there will be some conflicts with other updates, so I will let you know shortly.

I applied the change to lightning and tried to rebuild P2J. I got these errors:

compile:
    [javac] Compiling 1048 source files to /mnt/san/sata/gc/20121029/p2j/build/classes
    [javac] /mnt/san/sata/gc/20121029/p2j/src/com/goldencode/p2j/xml/XEntityImpl.java:166: cannot find symbol
    [javac] symbol  : method getNode()
    [javac] location: interface com.goldencode.p2j.xml.XEntity
    [javac]       Element elChild = handle.unwrapXEntity(hChild).getNode();
    [javac]                                                     ^
    [javac] /mnt/san/sata/gc/20121029/p2j/src/com/goldencode/p2j/xml/XEntityImpl.java:236: cannot find symbol
    [javac] symbol  : method getNode()
    [javac] location: interface com.goldencode.p2j.xml.XEntity
    [javac]       Element elChild = handle.unwrapXEntity(hChild).getNode();

Please fix the issues and upload a new zip as soon as possible. I want to test this today. Thanks.

I have applied it. We will have to rebuild P2J before testing can start.

The remaining attribute/methods list has been updated after reviewing the 11.x Progress docs.

The update evl_upd20130206c.zip has been passed the regression testing ans was committed in bzr as 10166.

I have found that the following are actually in use in the application and need to be added for milestone 4:

Attributes:

NODE-VALUE
SUBTYPE

Methods:

INSERT-BEFORE
SET-ATTRIBUTE-NODE

Please prepare an update that adds these.

The update looks good. We will try to get it scheduled soon, but you will probably have to merge up to the bzr head before we can do that, since the appserver update and probably one for triggers will probably go first.

I have looked into current bzr. There is one point need to be discussed. The attribute SUBTYPE has implemented as standalone wrapper in handle class. But only as Readable attribute, while it is Read/Write one. I have extended XEntity:
...
public interface XEntity
extends XCommon, SubTypeAttribute
...
so every XEntity implementation will support getter from SubTypeAttribute interface. The setter methods are declared within XEntity itself. This way we preserve the current logic of the SubTypeAttribute interface creation with the new methods (XEntityImpl):
...
public character getSubType() - from SubTypeAttribute interface
public void setSubtype(character value) - from XEntity interface
public void setSubtype(String value) - from XEntity interface
...
Note the difference in method names. The question: What is better for this case either (get|set)SubType or (get|set)Subtype? I have used setSubtype in XEntity but we can mix both or use unified names.

The update looks good. I will start testing it shortly.

The update evl_upd20130222a.zip passed the conversion test and committed in bzr as 10184.

ca_upd20130308d.zip has passed conversion regression testing.

I'm fine with the change. Commit and distribute it.

Committed to bzr revision 10267

• com.goldencode.p2j.xml.XEntityImpl#getUniqueID
• com.goldencode.p2j.xml.XEntityImpl#getNamespaceURI
• com.goldencode.p2j.xml.XNodeRefImpl#getLocalName
  Also pools in com.goldencode.p2j.xml.XmlFactory is not implemented too.

• ADD-SCHEMA-LOCATION (XCommon is a good place for this method)
• CLONE-NODE (XEntity is a good place for this method)
• GET-ATTRIBUTE-NODE (XCommon is a good place for this method)
• GET-PARENT (XEntity is a good place for this method)
• IMPORT-NODE (XEntity is a good place for this method)
• INITIALIZE-DOCUMENT-TYPE (XDocument is a good place for this method)
• LONGCHAR-TO-NODE-VALUE (not sure about where is the best place: there are to classes XNodeRef and XmlFactory, I think XmlFactory is the best choice for it)
• MEMPTR-TO-NODE-VALUE (not sure about where is the best place: there are to classes XNodeRef and XmlFactory, I think XmlFactory is the best choice for it)
• NODE-VALUE-TO-LONGCHAR (not sure about where is the best place: there are to classes XNodeRef and XmlFactory, I think XmlFactory is the best choice for it)
• NODE-VALUE-TO-MEMPTR (not sure about where is the best place: there are to classes XNodeRef and XmlFactory, I think XmlFactory is the best choice for it)
• NORMALIZE (XEntity is a good place for this method)
• REMOVE-ATTRIBUTE (XCommon is a good place for this method)
• REPLACE-CHILD (XEntity is a good place for this method)

• CHILD-NUM (XEntity is a good place for this method)
• ENCODING (XDocument is a good place for this method)
• LOCAL-NAME (NamespaceURI is a good place for this method)
• NAMESPACE-PREFIX (NamespaceURI is a good place for this method)
• NAMESPACE-URI (NamespaceURI is a good place for this method)
• PUBLIC-ID (I didn't see any proper interface to work with schema(DTD), maybe we need to create it ?)
• SYSTEM-ID (I didn't see any proper interface to work with schema(DTD), maybe we need to create it ?)
• TYPE (XDocument is a good place for this method)

• INSTANTIATING-PROCEDURE
• NONAMESPACE-SCHEMA-LOCATION
• SCHEMA-LOCATION
• SCHEMA-PATH
• SUPPRESS-NAMESPACE-PROCESSING
• ADD-SCHEMA-LOCATION

2) Also there are 4gl methods that aren't implemented yet:

What do you mean by this? They are stubbed out already. For example, ADD-SCHEMA-LOCATION is defined as 4 methods in XCommon and there are stubs for these in XCommonImpl.

3) Also there are attributes that aren't implemented yet:

These are stubbed out too.

What P2J bazaar revision are you looking at? I think it may be old.

Next attribute only for version of 4gl 10+(ABL) do we need to do it ?

I'm not sure what you mean by this. We prefer to implement all features unless it will take so much extra time that we cannot afford it. The customer application runs on 10.2B so v10 features are very important, although specific features may not be strictly needed for the customer server project.

Greg Shah wrote:

2) Also there are 4gl methods that aren't implemented yet:

What do you mean by this? They are stubbed out already. For example, ADD-SCHEMA-LOCATION is defined as 4 methods in XCommon and there are stubs for these in XCommonImpl.

It was misunderstanding, I mean that this method doesn't have runtime implementation(at least some of them), only stubs. But for example CLONE-NODE is not implemented, I found mention only in common-progress.rules and SignatureHelper class. Maybe I don't understand, but where I should implement for example CLONE-NODE method, or I should do the same like ADD-SCHEMA-LOCATION method.

Also there are attributes that aren't implemented yet:

These are stubbed out too.

Same, I found mention only in common-progress.rules and SignatureHelper, but didn't find any mapping for this methods like for ADD-SCHEMA-LOCATION.

What P2J bazaar revision are you looking at? I think it may be old.

I use up to date bzr revision.

OK, please make a list of the following:

1. Method stubs that exist but need an implementation written.
2. Methods that are not yet stubbed.
3. Attribute stubs that exist but need an implementation written.
4. Attributes that are not yet stubbed.
5. Any other features that need implementation.
6. Design questions and/or ideas.

1. Method stubs that exist but need an implementation written.

2. Methods that are not yet stubbed.

3. Attribute stubs that exist but need an implementation written.

4. Attributes that are not yet stubbed.

5. Any other features that need implementation.

Also pools in com.goldencode.p2j.xml.XmlFactory is not implemented too

6. Design questions and/or ideas.

My plan is:
1) implement stubs
2) make stubs for methods/attributes which don't have stubs, put stubs in proper interfaces
3) implement stubs from 2)
4) write testcases

Any current portions of the implementation must be considered incomplete and potentially incorrect. We did not really spend any serious time trying to implement the code. For example, in XDocumentImpl:

   public logical getDocumentElement(handle hRoot)
   {
      hRoot = getHandle();
      if (valid())
      {
         return new logical(true);
      }
      else
      {
         return new logical(false);
      }
   }

This can't be correct. It essentially doesn't do anything. I think it should be like this:

   public logical getDocumentElement(handle hRoot)
   {
      hRoot.assign(getHandle());
      return new logical(valid());
   }

We were in a rush to get the overall support stubbed out in a reasonable way. Now we must carefully consider doing a proper implementation. There may be many places that are not listed as TODO, but still need work. Another example:

   public logical load(character mode, Object source, logical validate)
   {
      String loadMode = mode.getValue();

      if (loadMode.equalsIgnoreCase("file"))
      {
         try
         {
            xdoc = XmlHelper.parse((String)source, validate.getValue(), null);
         }
         catch (Exception e)
         {
//            throw new IOException("Error reading XML document from file", e);
         }
      }
      else if (loadMode.equalsIgnoreCase("memptr"))
      {
         return new logical(false);
      }
      else
      {
         return new logical(false);
      }

      return new logical(true);
   }

The memptr case is not implemented and the file case needs proper error handling.

Please re-examine the classes with this in mind.

We implement the Progress HANDLE using the handle.java class. This is a generic (type-less) "reference" to some kind of 4GL resource that can be passed around and it can be dereferenced to read/write an ATTRIBUTE or to call a METHOD. In Java the resources that can be contained in a handle must implement the WrappedResource interface. XCommon extends this interface, so any XCommon instance can be set into a handle (using handle.assign() or new handle(xcommon-ref). And then we can dereference that handle and get the contained wrapped resource and call the Java methods that implement the 4GL attributes or 4GL methods.

All of the SAX and DOM support in the 4GL is handle based. This means that we have to determine the proper mapping between our object hierarchy and the 4GL resources.

The initial design (see XEntityImpl) creates a map that tries to match handles to DOM Element instances. I really don't want to do it this way because this is extra complexity in the maintenance of the tree. We only need this if the 4GL somehow has some persistent reference to DOM elements as the same handle over and over. Can we get rid of the map? If not, I think that our maintenance of the map is not 100% so that needs careful review.

I would like you to review the overall design and determine any changes.

Finally, we have found (through many years of pain and mistakes) that the toughest part of this work is actually knowing what needs to be implemented. The 4GL is a strange and nasty beast. It is very common for the documentation to be incomplete and/or incorrect in many places. In addition, while the Progress developers may not have considered that a 4GL programmer would need to know some details about the implementation, we certainly do need the full details on how something really works. We have to determine IN ADVANCE, all the error processing, boundary conditions, quirky/unexpected behavior and hidden default behavior. Please write your testcases first. Run them and learn how this stuff works. Then enhance them to make sure that all of the above error processing, boundary conditions... are known. With that, you will find that the implementation becomes straightforward.

The initial design (see XEntityImpl) creates a map that tries to match handles to DOM Element instances. I really don't want to do it this way because this is extra complexity in >the maintenance of the tree. We only need this if the 4GL somehow has some persistent reference to DOM elements as the same handle over and over. Can we get rid of the map? If not, >I think that our maintenance of the map is not 100% so that needs careful review.

I suggest to use org.w3c.dom.Node.setUserData(String key, data, handler); method to store handle of node. It's gives us opportunity to remove map from code.

This is a good idea. But before we take on that additional complexity, I would like to know the answer to this question:

Does the 4GL always provide the same handle back for a given node? One way of detecting this is by using the STRING built-in function to render the handle as a kind of unique hash code. This will let you compare the handle instances for each node and if you access (via some kind of searching or enumeration) the same node multiple times and it always gives you back the same handle instance, then presumably someone could write code to depend on this implementation feature.

Document the results of your testing here. In the case where we do need to have a 1 to 1 mapping of handle instances to specific DOM elements, then we will use your approach.

-------- Original Message --------
Subject: small change in xml module
Date: Thu, 25 Apr 2013 11:34:09 +0300
From: Ovidiu Maxiniuc <om@goldencode.com>
To: Evgeny Kiselev <evk@goldencode.com>
CC: Greg Shah <ges@goldencode.com>

Evgeny,

While working on the int64 datatype runtime I found a small code that
need to be update in XEntityImpl.java to work with new datatype.

Please change the first line of public logical getChild(handle hChild,
integer index) to
int ndx = index.intValue();
instead of
int ndx = index.getValue();

Also, there it might be useful to check first if index is not unknown,
in which case probably you should return the unknown value.
This is how Progress does usually, but you need to check its behavior first.

Thanks,
Ovidiu

That is why I was wondering if the mapping was needed. If you can't find any need, then get rid of it.

Do we even need to store the hXEntity at all? Can't we just return a new handle(this) whenever the caller needs a handle to the current XEntity? Again: the key question here is whether there is a way in the 4GL to encode a dependency upon the specific handle instances being found (e.g. via a walk of the DOM tree).

Evgeny, you need to be real careful when moving/deleting/adding methods (which map 4GL methods and attributes) in the P2J interfaces. This is because, when accessing a certain attribute/method via a handle, and that method can be used to more than one resource, the P2J conversion can't determine what is the runtime resource referenced by that handle.

In your update, I see that you have moved some XCommon.removeAttribute APIs to SaxAttributes, but the REMOVE-ATTRIBUTE still converts to handle.unwrapXCommon(h).removeAttribute() - which is not OK, as in some cases the runtime will fail, as XCommon interface is not a SaxAttributes.

Evgeny: when you add a new interface for methods/attributes and use that interface in methods_attributes.rules (for the hwrap variable), you need to add an unwrap API in the handle class (i.e. handle.unwrapXmlSchema for your XmlSchema case).

Do I need to implement delete() and resourceDelete() methods ? Because I did't find any implementation of this methods in any other classes.

The HandleChain.delete should look like this:

   public void delete()
   {
      // call resource-specific delete
      resourceDelete();
      // if not chained, exit
      if (!(hasPrevSibling() && hasNextSibling())
      {
         return;
      }
      // else, remove it from the chain (i.e. change the prevSibling's and nextSibling's references accordingly)
      ...
   }

For the resourceDelete() implementation, you need to determine how your resources act when DELETE OBJECT is called for the handle:
- is the handle set to unknown?
- if the same resource is referenced by more than one handle, deleting one handle alters the resource in the other handle? Also, after one handle is deleted, are the handles still equal? What does VALID-HANDLE return for a deleted handle?

def var h1 as handle.
def var h2 as handle.
/* create h1 with some resource */
h2 = h1.
message h2 = h1 h1 h2 valid-handle(h1) valid-handle(h2).
delete object h2.
message h2 = h1 h1 h2 valid-handle(h1) valid-handle(h2).

I still have a problem with delete statement. Aast generate java code:
HandleOps.delete(hDoc);
But inside delete method, there is a code:

if (referent instanceof Deletable)
{
    ((Deletable) referent).delete();
}

throw new RuntimeException(
    "Delete implementation not available for handle " + 
        handle.unwrapType(h).getResourceType().toStringMessage());

Code Review (0527a)

1. Almost all of the files are missing a history entry describing your changes.

2. handle.java unwrapEncoding() javadoc should reference Encoding instead of XmlSchema.

3. At the end of a class, please don't place an extra blank line after the last method.

For example, this:

   public void something();

}

should be this:

   public void something();
}

And this:

   public void something()
   {
      ...
   }

}

should be this:

   public void something()
   {
      ...
   }
}

4. The Encoding.java class needs class-level javadoc. Please also enhance the method javadoc to better describe what exactly the "encoding" is, what constitutes valid/invalid input and how errors are handled.

5. The methods_attributes.rules changes should be placed in the file alphabetically (by language keyword). We don't group things by function there because it is too hard to find things later that way.

6. When writing TRPL rules, although we accept the token keywords as upper or lowercase (or indeed as mixed case), it is the convention to always write them in lowercase. Please change prog.KW_NORMALZE as prog.normalze.

7. Data and static initializers in XmlFactory should be placed at the top of the file (in accordance with our coding standards/conventions).

8. The data members and methods currently at the bottom of XmlFactory need Javadoc. Actually, I see many places in the code that need javadoc. We don't check any code in that doesn't have all methods and members fully javadoc'd. This is part of our coding standards/conventions.

9. The string processing in XmlFactory probably needs to be case-insensitive. For example, in the 4GL string comparisons are typically case insensitive. I suspect that when 4GL code passes text like "ELEMENT" it can also be "element" or "eLeMenT".

10. I see quite a few TODO entries. Obviously, the ones related to SAX are not an issue. But I see TODOs related to DOM stuff. For example, the "pool" functionality in XmlFactory is missing.

11. The XmlFactory.createXNodeReference() implementation (both versions) looks wrong. First, the caller is passing in a Document object but we don't want the J2SE DOM classes exposed to the converted 4GL code. I think this should be a handle that contains an XDocument, right? The type string processing in the namespace form seems wrong (it only checks to see if the character variable is unknown - is that really how it is done in the 4GL?). Another issue is that it seems too easy to get a null value for the node. This just doesn't look complete.

12. Generally, I don't see the kind of boundary checking and error processing that I expect will be needed to make this fully compatible. This concern is throughout the code. Examples:

• There seem to be many places where DOM processing is called and the results are not checked for null or validity.
• In many places the 4GL probably has ways of returning unknown value but it seems this is almost never returned.
• I am sure the 4GL has a long list of error messages (and ERROR conditions that should be raised). These things usually must be raised with ErrorManager.recordOrThrowError(). XDocumentImpl is the one exception to this (it has some 4GL compatible error processing).
• There are places where the J2SE exceptions are re-thrown. That cannot ever be done. Everything must be wrapped in on one of the ConditionExceptions. If something is truly supposed to be fatal, then it is often a StopConditionException, but that is VERY RARE and I wouldn't expect the XML processing to generate a fatal error (that ends the 4GL session). The converted code must not ever see J2SE exceptions.

13. I see places in the code which break the coding standard of putting all open/close curly braces on their own line.

It is important for you to take the following approach: ALWAYS write your 4GL testcases FIRST. It is very important to find all the "features", quirky behavior, error handling and boundary conditions present in the 4GL. Usually, it is surprising how much of that stuff is there. It is usually all undocumented, so just reading the 4GL reference is not enough. You must write 4GL code that is expected to fail and/or where you try to break it in every possible way. Use features like NO-ERROR, ERROR-STATUS:ERROR and ERROR-STATUS:GET-NUMBER to write code that exposes error behavior in a way that allows the program to continue running. Please see testcases/uast/memptr/memptr_start_get_before_beginning.p for a good example of this. In fact, please browse through that memptr/ directory to look at the variety of testcases I created. You will see the extent to which we must try to break things in order to learn exactly how the 4GL works. Note that I also make it a practice to save off enough information in the comments and the code so that someone reading the testcase later should be able to know what it is supposed to do in the 4GL. I have a few .p files in that directory that don't conform fully to that ideal, but otherwise it is quite good. We have found that doing that work up front will save a great deal of time later when you are implementing the Java replacement. You will still find some things later that you didn't think of investigating up front, but believe me, this is the best way to do it. Trying to write an implementation first (based on the 4GL docs) and then fixing it later, will take MUCH longer.

Evgeny Kiselev wrote:

I still have a problem with delete statement. Aast generate java code:
HandleOps.delete(hDoc);

OK, there is a bug when deleting resources. I've missed a return after the delete() call; please add it to make it look like this:

            if (referent instanceof Deletable)
            {
               ((Deletable) referent).delete();
               return;
            }

Also HandleChain.delete() method is empty too, should I implement my own delete in child classes or implement common version from Constantin advice in №65 ?

Please go ahead and add only this code to HandleChain.delete; your resources don't support chaining, so I'll implement the TODO: as part of my work (for some resources which do support chaining).

  public void delete()
   {
      // call resource-specific delete
      resourceDelete();
      // if not chained, exit
      if (!(hasPrevSibling() && hasNextSibling())
      {
         return;
      }
      // TODO: else, remove it from the chain
   }

Greg Shah wrote:

Code Review (0527a)

11. The XmlFactory.createXNodeReference() implementation (both versions) looks wrong. First, the caller is passing in a Document object but we don't want the J2SE DOM classes exposed to the converted 4GL code. I think this should be a handle that contains an XDocument, right? The type string processing in the namespace form seems wrong (it only checks to see if the character variable is unknown - is that really how it is done in the 4GL?). Another issue is that it seems too easy to get a null value for the node. This just doesn't look complete.

I have question about XmlFactory.createXNodeReference():
This method is used only in com.goldencode.p2j.xml.XDocumentImpl.createNode() and com.goldencode.p2j.xml.XDocumentImpl.createNodeNamespace() and is not exist in generated code(I mean call to XmlFactory is hidden in XDocumentImpl). I just want to understand a border where I can use something from other packages or API and where I shouldn't do it.

The basic idea here:

Converted                          P2J Runtime                            P2J Runtime
Code                               Public API                        Internal Implementation
---------------                  ------------------                  -----------------------
handle(s) to                      interfaces like                     direct access to DOM
XML resources      <-------->     XDocument            <-------->     (e.g. Document, Node...)

The internal implementation should not be made public. The only stuff that is public is stuff that operates using handles and the interfaces like XDocument that are unwrapped and called from converted code.

I have found next issue with memptr:
Caused by: java.lang.NullPointerException
at com.sun.proxy.$Proxy3.isLittleEndian(Unknown Source)
at com.goldencode.p2j.util.memptr$1.initialValue(memptr.java:96)
at com.goldencode.p2j.util.memptr$1.initialValue(memptr.java:90)
at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:172)
at com.goldencode.p2j.util.memptr.setLength(memptr.java:466)

IS it issue with my environment ?

IS it issue with my environment ?

Possibly. The code being called is native (JNI) in src/native/memory.c:

JNIEXPORT jboolean JNICALL
Java_com_goldencode_p2j_util_MemoryManager_isLittleEndian(JNIEnv* env, jclass cls)

That code should not be able to generate an NPE. It is very simple and self-contained.

The more likely reason is that the remote access to the client from the server is somehow broken. Is your libp2j.so properly built and found in the java.library.path?

Thanks, it was issue from my side.
But I have next issue. I have a test:

set-size(m1) = 10001.
set-size(m2) = 101.
do i=1 to 10000:
put-byte(m1, i) = i.
end.
hChild:memptr-to-node-value(m1).
hChild:node-value-to-memptr(m2).
if m1 <> m2 then message "Unexpected error memptr-to-node-value and node-value-to-memptr".


It works fine in the 4GL, but in p2j I have next error: ** Can't PUT past the end of the MEMPTR. (4791)
I'm not sure about decision. Should I extend memptr in my code (node-value-to-memptr method) or it should works automatically ?

It works fine in the 4GL, but in p2j I have next error: ** Can't PUT past the end of the MEMPTR. (4791)

Do you get that on the line with PUT_BYTE()? If so, what is i at the time?

If not, then where does the error occur?

Should I extend memptr in my code (node-value-to-memptr method) or it should works automatically ?

I need to know more before I can say.

I have one more issue: in the generated code I have: unwrap(hRoot).readOnlyError("namespace-prefix");
but I'm expected to see handle.unwrapXEntity(hRoot).setNamespacePrefix("some_data");
Here is mapping:

<rule>ftype == prog.kw_namesp_p
  <action>hwrap = "XEntity"</action>
  <action>methodText = "getNamespacePrefix"</action>
  <rule>isAssign
     <action>methodText = "setNamespacePrefix"</action>
  </rule>
</rule>

Greg Shah wrote:

It works fine in the 4GL, but in p2j I have next error: ** Can't PUT past the end of the MEMPTR. (4791)

Do you get that on the line with PUT_BYTE()? If so, what is i at the time?

If not, then where does the error occur?

The error comes from method com.goldencode.p2j.util.BinaryData.setMethodSetup() there is a line that check if (isAutoExtend()) for memptr this method return always false.
than it throws error:

boolean bypass = (idxQuirk && size < 0) || (size == 0);

            if (!bypass)
            {
               genOutOfBoundsError();
               return false;
            }

Evgeny Kiselev wrote:

I have one more issue: in the generated code I have: unwrap(hRoot).readOnlyError("namespace-prefix");
but I'm expected to see handle.unwrapXEntity(hRoot).setNamespacePrefix("some_data");

Please add the KW_NAMESP_P token to the read_only_attribute rule in the common-progress.rules file; the read_only_attribute maintains a list of all writable attributes (as the default assumption is all attributes are read-only). Make sure to put the test in the proper place (lexicographical order).

Evgeny

But I have next issue. I have a test:

set-size(m1) = 10001.
set-size(m2) = 101.
do i=1 to 10000:
put-byte(m1, i) = i.
end.
hChild:memptr-to-node-value(m1).
hChild:node-value-to-memptr(m2).
if m1 <> m2 then message "Unexpected error memptr-to-node-value and node-value-to-memptr".


It works fine in the 4GL, but in p2j I have next error: ** Can't PUT past the end of the MEMPTR. (4791)
I'm not sure about decision. Should I extend memptr in my code (node-value-to-memptr method) or it should works automatically ?

Have you tried this on lindev01 too? I ask in case windev01 doesn't "see" the buffer overflow... and AFAIK linux is more strict on buffer overflow problems.

The error comes from method com.goldencode.p2j.util.BinaryData.setMethodSetup() there is a line that check if (isAutoExtend()) for memptr this method return always false. than it throws error

I need to know what the stack trace looks like in this case. In particular: what line of 4GL code corresponds to the code being processed in P2J that fails. Is it the PUT-BYTE call or not? If not, which line?

Greg Shah wrote:

The error comes from method com.goldencode.p2j.util.BinaryData.setMethodSetup() there is a line that check if (isAutoExtend()) for memptr this method return always false. than it throws error

I need to know what the stack trace looks like in this case. In particular: what line of 4GL code corresponds to the code being processed in P2J that fails. Is it the PUT-BYTE call or not? If not, which line?

Oh, sorry for my misunderstanding. hChild:node-value-to-memptr(m2). this line produce error. PUT-BYTE works fine, but a little bit slow.

How I can make attribute readonly via rules. I have kw_chld_num attribute, I have tried to find something in common rules (like Constantin advice in comment 83 with KW_NAMESP_P), but didn't find anything helpful. I don't like to add empty stub and just specify readOnlyError.

Evgeny, what rules have you tried and it didn't work? In 10364 rev, adding this rule to methods_attributes.rules (under the rule starting at 962):

               <rule>ftype == prog.kw_chld_num
                  <action>hwrap = "XEntity"</action>
                  <action>methodText = "getNumChildren"</action>
               </rule>

I don't like to add empty stub and just specify readOnlyError.

The idea is that writing to such an attribute is a compile-time error. If we find such problems in the code, they should be fixed in the original 4GL source code.

Greg Shah wrote:

I don't like to add empty stub and just specify readOnlyError.

The idea is that writing to such an attribute is a compile-time error. If we find such problems in the code, they should be fixed in the original 4GL source code.

Actually, the error is not at compile-time, but at runtime... that is why we are emitting a handle.readOnlyError API call for attributes known to be read-only and used as a l-value in assignments.

What I mean by "compile time" is that it can only be fixed by changing the code and re-compiling.

Yes, this manifests in the 4GL as a runtime failure, which is why we duplicate that behavior.

May I change in method handle.readOnlyError last line:
ErrorManager.displayError(4052, err, false);
to
ErrorManager.recordOrThrowError(4052, err, false);

Because in my test hDoc:name="asd" no-error. and I used handle.readOnlyError and I expected do not see error in GUI but I still received error in GUI.

Because in my test hDoc:name="asd" no-error. and I used handle.readOnlyError and I expected do not see error in GUI but I still received error in GUI.

I don't understand. We don't yet have a GUI in P2J, do you mean the ChUI (character UI) in our Swing client? Or are you referring to the use of the 4GL GUI client?

In regard to the proposed change, it is most important to test in the original Progress 4GL environment (ChUI and GUI). In the 4GL, if access to a read-only attribute while using NO-ERROR is silent AND records the data into ERROR-STATUS handle, then your change makes sense.

But I would have expected Constantin to implement that way if all read-only attributes worked that way. So I am worried that your read-only attributes are working differently than the ones Constantin tested. Please confirm the behavior in the 4GL of your read-only attributes and let us know.

Greg Shah wrote:

I don't understand. We don't yet have a GUI in P2J, do you mean the ChUI (character UI) in our Swing client? Or are you referring to the use of the 4GL GUI client?

I mean ChUI.

In regard to the proposed change, it is most important to test in the original Progress 4GL environment (ChUI and GUI). In the 4GL, if access to a read-only attribute while using NO-ERROR is silent AND records the data into ERROR-STATUS handle, then your change makes sense.

I have tested it in original Progress 4GL environment(only ChUI).

But I would have expected Constantin to implement that way if all read-only attributes worked that way. So I am worried that your read-only attributes are working differently than the ones Constantin tested. Please confirm the behavior in the 4GL of your read-only attributes and let us know.

Yes, you are right, I have found it only for one case. Next code working without any exceptions in 4GL environment, but not working in p2j.

/*test name attribute X-NODEREF*/
function nameAttribute returns int (err-num as int):
   def var hDoc as handle.
   def var hRoot as handle.
   create x-document hDoc.
   create x-noderef hRoot.
   hDoc:create-node(hRoot,"root","element").

   hDoc:name = "asd" no-error.
   if not error-status:error and not error-status:get-number(1) eq 4052
               then message "Unexpected error name: " error-status:get-number(1) " " error-status:get-message(1).

   hRoot:name = "asd" no-error.
   if not error-status:error and not error-status:get-number(1) eq 4052
            then message "Unexpected error name: " error-status:get-number(1) " " error-status:get-message(1).

   return 0.
end.

nameAttribute(-1).

2 thoughts:

1. Your test code needs a slight change. Unfortunately, when I pointed you to my memptr test code as examples, I had a bug in the error checking logic. I have since fixed that, but I see that my error has crept into your code too.

if not error-status:error and not error-status:get-number(1) eq 4052

should be changed to:

if not error-status:error OR not error-status:get-number(1) eq 4052

Otherwise a non-error case won't fail the test.

2. Please try this read-only test on some other read-only attribute (preferably one that is not XML based), to see if the behavior matches your testcase or not. If other read-only attribute behavior is all the same, then a change to the core readOnlyError() method is the proper solution. However, if you have found a behavior that is slightly different from other read-only attributes, then we will have to implement a variant of readOnlyError() with different behavior and then we will have to select the proper variant during conversion, depending on the attribute (and possibly the resource).

I have found that for most of the DOM methods and attributes exceptions doesn't set error=true.
I have tried to use ErrorManager.addError(9082, err, false); but in the method ErrorManager.silentErrorDisable(); there is a code:

if (!pending)
         da.clearList();

Any of the methods listed above may fail, but this does not normally raise the ABL
ERROR condition. Instead, the method returns FALSE if that is appropriate. Also, the
parsing may encounter errors that do not cause the operation as a whole to fail. So
instead of testing for ERROR-STATUS:ERROR after running a method with NO-ERROR, you
should test for ERROR-STATUS:NUM-MESSAGES being greater than zero.

Yes, go ahead with this approach.

Dom, Sax and many other objects has handle attribute. I didn't find any implementation for this. Also conversion for this attribute doesn't work properly. As I can see, this is global attribute, should I need to implement it in current task ?
For example:
hRoot:handle = hDoc no-error.
produce:

ErrorManager.silentErrorEnable();
hRoot;
"handle";
ErrorManager.silentErrorDisable();

It is a global attribute, but the DOM XML support is one of the first areas of 4GL code where we have found a good reason to use this approach. We do handle this for the "system handles" already, but we don't have support for non-system handles.

To see the locations where we have support (and the non-system handle location that is commented out), look for prog.kw_handle in rules/convert/methods_attributes.rules.

The general idea is that we are already explicitly emitting handle instances for resources that the 4GL does not explicitly represent as handles (e.g. widgets), so we should not need much to support the DOM requirement.

I guess the result should look something like this:

ErrorManager.silentErrorEnable();
hRoot.assign(hDoc);
ErrorManager.silentErrorDisable();

The no-error support is already there. There are 2 issues to be addressed:

1. The hDoc instance must be emitted. Actually, I am surprised it isn't emitted already.
2. The handle attribute must be emitted as an assign() instance method call when it is used as an lvalue. I don't think we need to emit anything when it is used as an rvalue (or in an expression that is not an lvalue). The Java handle reference should already emit in that case. So this fix should only be in the assign case.

See if you can fix this. If it becomes too much of a problem, let me know and we will postpone it.

I have found error in 4gl on unix server(lindev01)(version oe10.2B) :
after execution of next line:
hDoc2:initialize-document-type(namespaceURI, ":asd", publicId, systemId).
I have received:

ERROR: Memory violation. (49)
** Save file named core for analysis by Progress Software Corporation. (439)

/*test initialize-document-type X-NODEREF*/
function initializeDocumentType returns int (err-num as int):
   def var hDoc2 as handle.

   def var namespaceURI as character.
   def var rootNodeName as character.
   def var systemId as character.
   def var publicId as character.

   create x-document hDoc2.

   namespaceURI = "http://www.w3.org/1999/xhtml".
   rootNodeName = "h:html".
   systemId = "http://www.w3.org/TR/html4/strict.dtd".
   publicId = "-//W3C//DTD HTML 4.01//EN".

   hDoc2:initialize-document-type(namespaceURI, ":asd", publicId, systemId) no-error.
   if error-status:error or error-status:num-messages <> 1 or not error-status:get-number(1) eq 9082
      then message "Unexpected error initialize-document-type 1: " error-status:get-number(1) " " error-status:get-message(1).

   return 0.
end.

initializeDocumentType(-1).

I think I do not need to simulate this scenario.

Yes, I think you are right.

The key question: was that code in the 4GL supposed to work (or were you recreating some kind of error input)?

Please post the latest code and a summary of the work that is left.

Evgeny, in regards to note 79. From the docs of node-value-to-memptr:

NODE-VALUE-TO-MEMPTR( ) frees the memory currently allocated by memptr (if any),
allocates sufficient memory to the MEMPTR to accommodate the node, and copies the node to
the MEMPTR.

Evgeny, in regards to note 100: have you gotten past this one? Can you post the full test program which you used to generate this error?

Constantin Asofiei wrote:

Evgeny, in regards to note 100: have you gotten past this one? Can you post the full test program which you used to generate this error?

Full test program was provided in note 100, I didn't check it on windows only on lindev01. This error appears after the completion of the program.

3) I have found strange case:
hRoot:GET-PARENT(hWork). if hRoot is unknown and hWork is valid and known widget, then ABL throws 9102 exception, but hWork become invalid.

I don't understand this one. If hRoot is unknown, then a Invalid handle. Not initialized or points to a deleted object. (3135) and a Cannot access the GET-PARENT attribute because the widget does not exist. (3140) errors are shown. How can hWork be set to unknown, as get-parent is never executed?

Evgeny, this the review for your 0707a.zip update. Please let me know if you see anything wrong with my reasoning.

1. Xmlfactory.createXNodeReference(handle, handle, character, character) - I think this is intended for internal usage, right? If so, please make it package private, to not expose it to the public. More, if docHandle is unknown/invalid or if a Document instance can't be found, shouldn't there be an error thrown? Because I think these cases should never happen and if they do, then normal processing should not be allowed, as it is a flaw in the runtime.

2. XNodeRefImpl.getOwnerDocument
- as a practice, when we expose BDT instances to the public, always return a copy of that BDT (because in most cases we need to expose them "by value"). In your case, as you directly expose the handle instance, the user can make a mistake and save this reference and change it in later code; by returning a copy, we ensure the user doesn't alter it.

3. XEntityImpl.invalidNodeError
- I don't like how the XEntityImpl.invalidNodeError is used throught the code. When a X-NODEREF resource is created, the resource IS valid (VALID-HANDLE returns true for it), but some methods can't be used until the X-NODEREF is attached to a document.
Lets look at XEntityImp.getName:

      if (unknown())
      {
         invalidNodeError("NAME");
         return new character();
      }

      if (!isAttached())
      {
         invalidNodeError("NAME");
         return new character();
     }

4. XEntityImpl.hXEntity and XEntity.getHandle
I don't like how this field and the API are used. More, I don't think the getHandle API should be defined at all, as the HANDLE attribute should be ignored by the conversion rules and act as if it is not even there (as a side note, there is a problem in converting h:handle:attribute or h:handle:method() chain calls, but this should not affect this task).
In P2J, if you need to obtain a handle instance for a resource, just use new handle(resource). This is because in 4GL handles act like pointers to a resource; the ID of a handle (obtained via string) will always be the ID of the resource; thus, two handles are equal as long as the referred resources are the same (or both handles are unknown). When you call DELETE OBJECT handle, you don't delete the handle, but the resource referred by the handle.
From what I see, the hXEntity is used to maintain a handle instance which refers the same resource instance to which hXentity belongs (as the XEntityImpl(WrappedResource) c'tor is never called in current code) - IMO, this is redundant and just makes the code more complicated.
Plus, I see that at some point hXEntity is made null in XEntityImpl.clear, but I can't find any calls to this method. And in XDocumentImpl.initializeDocumentType, the getHandle().assign(this) also looks redundant: unless clear() is called, hXEntity is never changed (btw, what was the purpose of XEntityImpl.clear()?.
To conclude, as the HANDLE attribute is always read-only and as you need a handle instance when calling Xmlfactory.createXNodeReference method (for the docHandle attribute), you can always add a asHandle() method for easy access to a handle referring this instance; but this should not be exposed to the public.

5a. XEntityImpl.insertBefore (and others)
This code (which validates the resource) is redundant at the beginnig of the method:

      if (unknown() || !valid())
      {
         return new logical(false);
      }

5b. XEntityImpl.insertBefore
The validation of the handle1 and handle2 parameters seems wrong to me:

      if (!handle1._isValid())
      {
         invalidArgumentError("INSERT-BEFORE");
         return new logical(false);
      }

      if (handle1.isUnknown() || handle2._isValid() && handle2.isUnknown())
      {
         invalidNodeError("INSERT-BEFORE");
         return new logical(false);
      }
      if (handle2 == null || handle2.isUnknown())
      {
         return appendChild(handle1);
      }

6. XEntityImpl.getChild
This is a little tricky. The docs state the hChild parameter must be a "valid X-NODEREF handle". This translates into: you should not replace the resource of hChild, but you should update the node referred by the resource (to qoute "we need to go deeper"). Current implementation changes the resource referred by the hChild handle, which (unless you found otherwise during testing) is incorrect.
I think the success return value should be directly true, not hchild.isValid()

7. XentityImpl.invalidArgumentError
- if you need access to the HandleResource.type field, why don't add a getter for it? Please name the getter type() (we don't want to collide with a getType defined in a DMO).

8. XNodeRefImpl.setAttributeNode(handle)
- this test is incorrect: XDocumentImpl.TYPE_X_DOCUMENT.equals(resource.getResourceType()) you check a String against a character which will always be false.

9. XNodeRef.getATtributeNode(handle, character)
Same with XEntityImpl.getChild; the handle must contain a valid X-NodeRef resource where to save the attribute; you are creating a new XNodeRef, which (unless proved otherwise by testing) is incorrect.

1. I see that you make (all?) local vars final; this makes the code harder to read, as when for a code like:
   

   final Something var = new Something();
   

   
   the reader would expect for var to be used in an anonymous inner class. You need to eliminate this from the code (note that is OK to use final local vars when they represent string error messages, int error codes or some other real constant case, as the reader can really see these as constants).
2. make sure the fields/methods are sorted by their modifiers
3. double check all methods/fields for javadoc and all files for history entries
4. XDocumentImpl.initializeDocumentType (and others) - the javadoc is not formatted according to standard; please double check all files (there are other cases which use the same, non-standard, formatting)
5. I see in your code cases like this:
   

   final result = /* some simple expression */
   return result;
   

   
   Why not use just return /*simple expression */; instead, as it is easier to read?
6. same for code like this:
   

               final String errmsg = e.getMessage();
               ErrorManager.recordOrThrowError(557, errmsg);
   

   
   why not use directly ErrorManager.recordOrThrowError(557, e.getMessage());? Easier to read.
7. when you have a string error message which can't be placed inline with the method call, as in:
   

               commonDomError("INSERT-BEFORE", "A node was used in a different document than " +
                       "the one the created it");
   

   
   please use either this:
   

               commonDomError("INSERT-BEFORE", 
                              "A node was used in a different document than the one the created it");
   
   

   
   or this version:
   

               commonDomError("INSERT-BEFORE",
                              "Really long message which needs two lines: A node was used in a " +
                              "different document than the one the created it");
   
   

8. avoid unnecessary argument listing on multiple row, as in:
   

         String err = String.format(msg,
                                    attribute.toUpperCase(),
                                    message);
   

   
   should be
   

         String err = String.format(msg, attribute.toUpperCase(), message);
   

9. XnodeRefImpl.normalize:
   

         if (nodeType == Node.ELEMENT_NODE || nodeType == Node.DOCUMENT_FRAGMENT_NODE
                 || nodeType == Node.ATTRIBUTE_NODE)
   

   
   should be:
   

         if (nodeType == Node.ELEMENT_NODE           || 
             nodeType == Node.DOCUMENT_FRAGMENT_NODE || 
             nodeType == Node.ATTRIBUTE_NODE)
   
   

10. XmlHelper.parse(InputStream, boolean, String) definition should be formatted like this:
    

       public static Document parse(InputStream in, boolean validate, String systemId)
       throws ParserConfigurationException,
              IOException,
              SAXException
    

11. XmlHelper.parse(String, boolean, String, boolean) - check the parameter formatting

1. ErrorManager.java, handle.java, methods_attributes.rules, common-progress.rules in the update are still not merged
2. why have you made XEntityImpl non-abstract? AFAIK, instances of this class should not exist.
3. I see that XEntityImpl.hXEntity is still there; if you are planning to further use it, please explain its usage. More, look at the call path from line 507 in XDocumentImpl.getDocumentElement(handle hRoot):
   - hRoot.assign(new XNodeRefImpl(hRoot, documentElement));
   - the hRoot parameter is a handle reference where the root node will be saved;
   - the callpath from the XNodeRefImpl(handle, Node) c'tor is XEntityImpl(handle, Node) => XEntity(handle) => XEntity(WrappedResource, handle)
   - the XEntity(WrappedResource, handle) c'tor has these lines:
   

         hXEntity = h == null ? new handle() : h;
         hXEntity.assign(value);
   

   
   In this c'tor, the handle variable is not-null and points to the same object as the hRoot parameter; thus, you save a reference to the same hRoot object passed to the XDocumentImpl.getDocumentElement call. This is not OK, as subsequent code after the XDocumentImpl.getDocumentElement call may change the resource referred by the hRoot handle which (as you save a reference to the same var) it will change the resource referred by the XEntity.hXEntity field. Here I'm not advocating in favor of the hXEntity field, but to show you that when saving references to BaseDataType instances, you need to be very careful.
   Finally, looking at the docs for GET-DOCUMENT-ELEMENT() method and how you've implement it, it has the same problem as points 6 and 9 at review in comment 107. Look at this test:
   

   def var hDoc as handle.
   def var h2 as handle.
   def var h3 as handle.
   create x-document hDoc.
   create x-noderef h2.
   create x-noderef h3.
   message h2 h3.
   hDoc:get-document-element(h2).
   hDoc:get-document-element(h3).
   message h2 h3. /* h2 and h3 point to the same resources as before the get-document-element calls */
   

   
   The resources referred by the h2 and h3 handles will not change; thus, get-document-element updates a state of the contained resource. Also, this is will fail:
   

   def var hDoc as handle.
   def var h2 as handle.
   create x-document as handle.
   h2 = session.
   hDoc:get-document-element(h2). /* error here as h2 is not a X-NODEREF*/
   

   
   Please review all methods for X-NODEREF and X-DOCUMENT resources and post here the list of methods which update a state of the resource, and not the handle's resource. For all these cases, you will need to validate the passed handle to refer an expected resource.
4. XDocumentImpl.createNodeNamespace lines 436-442 are formatted incorrect:
   

            if (nodeType == null
                    || !(
                        nodeType == XmlTokenTypes.ELEMENT_NODE
                        || nodeType == XmlTokenTypes.ATTRIBUTE_NODE
                        )
               )
   

   
   should have been:
   

            if (nodeType == null ||
                !(nodeType == XmlTokenTypes.ELEMENT_NODE ||
                  nodeType == XmlTokenTypes.ATTRIBUTE_NODE))
   

   
   or (if second line fits):
   

            if (nodeType == null ||
                !(nodeType == XmlTokenTypes.ELEMENT_NODE || nodeType == XmlTokenTypes.ATTRIBUTE_NODE))
   

5. XEntityImpl.getChildNum line 376:
   

         if (XDocumentImpl.TYPE_X_DOCUMENT.equals(getResourceType().toStringMessage()))
         {
            return new integer();
         }
   

   
   Why add subclass-specific tests here? Isn't it better to override getChildNum in XDocumentImpl?

Constantin Asofiei wrote:

7. XentityImpl.invalidArgumentError
- if you need access to the HandleResource.type field, why don't add a getter for it? Please name the getter type() (we don't want to collide with a getType defined in a DMO).

Good reason. I didn't do it before because I've tried do not changes API if is it possible to use it without changes.

10. formatting problems (only most obvious ones, I'll recheck after these are fixed):

1. I see that you make (all?) local vars final; this makes the code harder to read, as when for a code like:
   [...]
   the reader would expect for var to be used in an anonymous inner class. You need to eliminate this from the code (note that is OK to use final local vars when they represent string error messages, int error codes or some other real constant case, as the reader can really see these as constants).

It was my practice to make everything final if I'll not change this variable. There is a lot of holywars about this practice. And in old JDKs it helps GC to clean memory.
http://www.javapractices.com/topic/TopicAction.do?Id=23
http://stackoverflow.com/questions/306862/does-using-final-for-variables-in-java-improve-garbage-collection
I'll remove all unnecessary finals keyword from code.

Why not use just return /*simple expression */; instead, as it is easier to read?

1. same for code like this:

Yes, just for it is easier to read.

why not use directly ErrorManager.recordOrThrowError(557, e.getMessage());? Easier to read.

I have fixed it in last update, but ErrorManager.recordOrThrowError set error flag.

Other issues are reasonable and will be fixed asap.

Constantin Asofiei wrote:

3. XEntityImpl.invalidNodeError
- I don't like how the XEntityImpl.invalidNodeError is used throught the code. When a X-NODEREF resource is created, the resource IS valid (VALID-HANDLE returns true for it), but some methods can't be used until the X-NODEREF is attached to a document.
Lets look at XEntityImp.getName:

      if (unknown())
      {
         invalidNodeError("NAME");
         return new character();
      }

I don't understand this - shouldn't this check if the node is attached to a document? Why do you link "unknown" state with something which is related to some other distinct, unique, state for this resource? IMO, this should look like:

      if (!isAttached())
      {
         invalidNodeError("NAME");
         return new character();
     }

where isAttached would return true if the node is attached to a document (in X-NODEREF case) and always true for X-DOCUMENT case.
For the DOM/SAX-related resources, you can override unknown() to always return true (as the resource is not a container for another object which acts as the resource - like in the PersistentProcedure case).

Actually there are 3 state of X-DOCUMENT or X-NODEREF

0   def var hRoot as handle.                             handle hRoot = new handle();                
1   create x-noderef hRoot.                              XmlFactory.createXNodeReference(hRoot);
2   hDoc:create-node(hRoot,"root","element").            handle.unwrapXDocument(hDoc).createNode(hRoot, "root", "element");
3   hDoc:append-child(hRoot).                           handle.unwrapXEntity(hDoc).appendChild(hRoot); 

Evgeny Kiselev wrote:

in 1 instance is created. In my case I'm understand this state as unknown, but there is some methods like unique, type is legal to use in this state.

No. Add this line after your line:

message hRoot hRoot = ? valid-handle(hRoot).

in 2 node with specific type is created but do not attached, every method is legal to use, but some method could throw exception

Again, this is something you need to explicitly track inside the resource; cant use unknown.

1. uninitialized - when the X-NODEREF resource is created with CREATE X-NODEREF, but has no XML node.
2. initialized, not attached - the X-NODEREF resource has an attached XML node created via a CREATE-NODE API call.
3. attached - when the X-NODEREF has an XML node and is attached

Don't forget that for the same X-NODEREF resource, you can change its inner XML node (point 3 in comment 109).

As a side-note, in some files there are lots of whitespace changes. Please change the settings to your editor so that whitespace is not mass-removed automatically, as it makes harder to follow changes during review.

1. methods_attributes.rules - I see that you made the HANDLE attribute to be converted to getHandle(). From the handle_test.p program, I think your problem is a runtime issue: hRoot:handle = hDoc needs to convert to handle.readOnlyError, regardless of the underlying resource. Looking back, my conclusion is that I misstested the cases for handle.readOnlyError and handle.invalidAttribute - both should not set the ERROR-STATUS:ERROR attribute and should not throw any error. The behavior you found for the X-NODEREF and related resources applies to other resources too (I've tested misc attributes from other resources and is the same). I think is safe to remove your XComonImpl.readOnlyError and make the changes directly in handle class. If you have questions, let me know.

2. handle.java - the history order is incorrect (your 027 entry should be 029)

3. XCommon.java
- just whitespace changes, remove the file from the update.

4. XCommonImpl.java - you have a few methods with notes like overrided in XDocumentImpl. If only certain resources need to override this (as this API is not valid for all resources implementing the XCommon interface), then let the default implementation call handle.invalidAttribute (as this is the normal behavior in case of API access attempt for resources which don't implement it).

5. XDocument.java - for the methods with the signature longer than max line length, please place the parameters one per line, with the parameter names aligned.

   public logical initializeDocumentType(String namespaceUri, character rootNodeName,
                                         String publicId, String systemId);

   public logical initializeDocumentType(String    namespaceUri, 
                                         character rootNodeName,
                                         String    publicId, 
                                         String    systemId);

6. XEntityImpl.insertBefore - note 107/5b. A resource can't be valid and unknown at the same time. Please look again at how you use handle1 at lines 178 and 184 (same tests as in 107/5b). The same for handle2 parameter - this must never be null. I'm not saying the code is not doing the job, but is confusing.

7. XEntityImpl - I see that you override equals and hashCode: please explain why this is necessary, as I think the equality/hashCode are per resource, and not per "resource's inner referent", which can be DOM attribute/node/etc in our case. Considering the example at note 109/3, have you tested what happens if i.e. getAttributeNode is used to extract attribute node A in two different resources (R1 and R2), and after that both change the attribute to something else - in this case, does the change reflect in the original reference? Does R1 see R2's changes? What I'm trying to say, is the attribute node extracted as a referrence or as a copy? Note that this applies for other APIs too (getChild, etc).

8. XmlFactory
- import declarations are not according to standard
- missing javadocs for fields

9. XmlSchema - import declarations are not according to standard

10. XDocumentImpl
- fields are missing javadoc/not formatted properly.
- initializeDocumentType - javadoc is still not formatted properly

11. XNodeRefImpl.setAttributeNode (and others) - the "final" modifier is not fully cleaned.

Greg: there are some common attributes (like INSTANTIATING-PROCEDURE and UNIQUE-ID) for which the implementation can be extracted in a single location. I think these should be left behind and added after all resources are implemented, else we will end up duplicating code.

Widget Pool is not implemented in com.goldencode.p2j.xml.XmlFactory.createSaxAttributes() because I didn't find appropriate information about widget pools.

Please leave widget pooling aside for now; the support will be added at the same time whe the widget pooling runtime is added.

Greg: there are some common attributes (like INSTANTIATING-PROCEDURE and UNIQUE-ID) for which the implementation can be extracted in a single location. I think these should be left behind and added after all resources are implemented, else we will end up duplicating code.

Agreed.

Evgeny, I missed something from your tests; your cases are mostly something like this:

h:<method> no-error.
if error-status:error or error-status:num-messages <> 1 or not error-status:get-number(1) eq code

Evgeny, please provide the list of methods which, like get-attribute-node, require a valid x-noderef resource and change a field of that resource. Why I want this: in your code, you have this for XNodeRefImpl.getAttributeNode:

public logical getAttributeNode(handle handle, character attributeName)
{
   ...
   handle.assign(new XNodeRefImpl(attrNode));
   ...
}

public logical getAttributeNode(handle handle, character attributeName)
{
   ...
   XNodeRefImpl hRef = (XNodeRefImpl) handle.getResource();
   hRef.setNode(attrNode);
   ...
}

By having a list of methods which act like get-attribute-node we can be sure that all are implemented right. If you have questions, please let me now.

Constantin Asofiei wrote:

Evgeny, please provide the list of methods which, like get-attribute-node, require a valid x-noderef resource and change a field of that resource. Why I want this: in your code, you have this for XNodeRefImpl.getAttributeNode:

Here are methods that could change resource:

com.goldencode.p2j.xml.XNodeRefImpl.cloneNode
com.goldencode.p2j.xml.XNodeRefImpl.getParent
com.goldencode.p2j.xml.XNodeRefImpl.getAttributeNode
com.goldencode.p2j.xml.XEntityImpl.getChild
com.goldencode.p2j.xml.XDocumentImpl.getDocumentElement
com.goldencode.p2j.xml.XDocumentImpl.importNode
com.goldencode.p2j.xml.XDocumentImpl.getDocumentElement

Evgeny, a lot better, only a few issues left:

methods_attributes.rules:
- getHandle - please explain why you still need to convert HANDLE attribute to getHandle. Note that this attribute can appear for any resource type, not just DOM/XML. And your implementation assumes that only DOM/XML resources can have a HANDLE attribute. Idea is, the handle attribute should be ignored by the conversion rules. Please indicate the exact case you need this, as I the problem might be in the conversion rules, but somewhere else.

SaxEntityImpl.java:
- unknown method returns true. Shouldn't it be false? Doesn't affect DOM/XML implementation, but just a thought.

XCommonImpl.java
- your error message implementation I don't think is correct. Have you tried methods/attributes without NO-ERROR clause? When NO-ERROR clause is missing, the error must be shown on the terminal; the error is registered only in the case NO-ERROR is present. Throughout the code you use ErrorManager.addError and also in XCommonImpl you've overridden the handle.readOnlyError method; this will not show the message to the terminal, when NO-ERROR is not used. Please double check how you treat errors, in cases when NO-ERROR is missing.

Please review ErrorManager class. I'm not sure about silentErrorEnable and silentErrorDisable methods. Because errorFlag put of only if isPending() = true.

My opinion is that you don't need these changes (if you think you do, please post a small testcase to show this). And, have you tried using recordOrShowError(code, msg, false, false) instead of addError? This should have solved your issues...

XDocumentImpl:
- separate each field definition with an empty line
- line 444: please format the test instead of this:

         if (nodeType == null ||
                 !(nodeType == XmlTokenTypes.ELEMENT_NODE ||
                   nodeType == XmlTokenTypes.ATTRIBUTE_NODE))

         if (nodeType == null ||
             !(nodeType == XmlTokenTypes.ELEMENT_NODE || 
               nodeType == XmlTokenTypes.ATTRIBUTE_NODE))

            String err = String.forreturn new logical(true);mat(msg, (String)source);

      if (!isXNodeHandle(xNodeRef)       ||
          !isXNodeHandle(xSourceNodeRef) ||
          !((XEntityImpl) xSourceNodeRef.getResource()).isInitialized())

XEntityImpl:
- getHandle - see the notes about methods_attributes.rules
- package private methods must be before protected methods (the order is public, package private, protected, private).

XmlFactory:
- fields must be separated with an empty line

XnodeRefImpl:
- line 510 - left align the expressions please, as in:

      if (currentNode.getNodeType() == Node.ELEMENT_NODE &&
          handleNode.getNodeType() == Node.ATTRIBUTE_NODE)

XmlHelper
- update the copyright date, as in:

** Copyright (c) 2004-2013, Golden Code Development Corporation.

XmlPatternWorker:
- update the copyright date, as in:

** Copyright (c) 2005-2013, Golden Code Development Corporation.

Constantin Asofiei wrote:

methods_attributes.rules:
- getHandle - please explain why you still need to convert HANDLE attribute to getHandle. Note that this attribute can appear for any resource type, not just DOM/XML. And your implementation assumes that only DOM/XML resources can have a HANDLE attribute. Idea is, the handle attribute should be ignored by the conversion rules. Please indicate the exact case you need this, as I the problem might be in the conversion rules, but somewhere else.

You are right. I'll try to look how I can manage it inside rules.

XCommonImpl.java
- your error message implementation I don't think is correct. Have you tried methods/attributes without NO-ERROR clause? When NO-ERROR clause is missing, the error must be shown on the terminal; the error is registered only in the case NO-ERROR is present. Throughout the code you use ErrorManager.addError and also in XCommonImpl you've overridden the handle.readOnlyError method; this will not show the message to the terminal, when NO-ERROR is not used. Please double check how you treat errors, in cases when NO-ERROR is missing.

Please review ErrorManager class. I'm not sure about silentErrorEnable and silentErrorDisable methods. Because errorFlag put of only if isPending() = true.

My opinion is that you don't need these changes (if you think you do, please post a small testcase to show this). And, have you tried using recordOrShowError(code, msg, false, false) instead of addError? This should have solved your issues...

The main problem that we can't manage ERROR flag. We have use pending flag for this. Line 387 da.setError(pending);. recordOrShowError method is fine, but on on line 533 we set da.setPending(true); and after that ERROR flag is always set in silentErrorDisable(). That why I have use addError method, but I think it's incorrect. For example I have moved from silentErrorDisable() method

if (!pending)
         da.clearList();

XEntityImpl:
- getHandle - see the notes about methods_attributes.rules
- package private methods must be before protected methods (the order is public, package private, protected, private).

In ther coding_standarts.odt says that:

7.    all methods
1.    static initializers
2.    constructors
3.    public static (except main)
4.    protected static
5.    package private static
6.    private static
7.    public instance
8.    protected instance
9.    package private instance
10.    private instance
11.    main

You are right. I'll try to look how I can manage it inside rules.

Please provide the test you which uses the generated getHandle API.

The main problem that we can't manage ERROR flag.

OK, I see now what you mean. Please provide the example you are using to test these changes.

In ther coding_standarts.odt says that:
[...]
Does it changed ?

Thanks for reminding me, my mistake, for some reason I thought package private goes first.

Constantin Asofiei wrote:

You are right. I'll try to look how I can manage it inside rules.

Please provide the test you which uses the generated getHandle API.

This code works fine in 4gl:

function handleTest returns int (err-num as int):
   def var hDoc as handle.
   def var hRoot as handle.
   create x-document hDoc.
   create x-noderef hRoot.

   hRoot:handle = hDoc no-error.
   if error-status:error or error-status:num-messages <> 1 or not error-status:get-number(1) eq 4052
            then message "Unexpected error handle: " error-status:get-number(1) " " error-status:get-message(1).

   hDoc:create-node(hRoot,"root","element").
   if not hRoot:handle eq hRoot then message "Unexpected error: hRoot".
   if not hRoot:handle:subtype eq "ELEMENT" then message "Unexpected error: hRoot".

  return 0.
end.

handleTest(-1).

The main problem that we can't manage ERROR flag.

OK, I see now what you mean. Please provide the example you are using to test these changes.

Example:

def var hDoc as handle.
def var hRoot as handle.
create x-document hDoc.
create x-noderef hRoot.

hRoot:subtype no-error.

message error-status:error.
message error-status:num-messages.
message error-status:get-number(1).

no
1
9102

About HANDLE attribute: please add this code in methods_attributes.rules:

               <!-- let it have a methodText, so that it can emit to readOnlyError, when is on the
                    left-side of an assignment. else, it will be ignored. -->
               <rule>ftype == prog.kw_handle and isAssign
                  <action>hwrap = ""</action>
                  <action>methodText = "readOnlyError"</action>
               </rule>

About ERROR flag. I still think that we need to use the existing ErrorManager.recordOrShowError, but we need to add a parameter to set the "pending" flag. The good news is that if an error is caught with NO-ERROR but previous code, subsequent code will clear the ERROR flag:

def var hDoc as handle.
def var hRoot as handle.
def var i as int.

create x-document hDoc.
create x-noderef hRoot.

function f0 returns char.
  return "not-an-int".
end.

i = dynamic-function("f0") no-error.
message error-status:error. /* this is yes */

hRoot:subtype no-error.

message error-status:error. /* this is no */
message error-status:num-messages.
message error-status:get-number(1).

we need to add yet another parameter isError, to the ErrorManager.recordOrShowError, so that the pending flag will not be set (isError will be true by default).

OK, do it.

Constantin's ErrorManager change will be checked in before your changes. So you don't have to include this file in your modifications.

Can you get your update ready for review today? If not, when can it be ready?

Evgeny, please merge your handle.java file, discard the ErrorManager changes you made and put the update through regression testing. The ErrorManager problem is a little bit more complicated than I thought, I'll handle it later today or tomorrow. The ErrorManager.recordOrShowError API usage will remain the same.

The idea is that we need to add a flag to "do not clean the error list", when recordOrShowError is used with a false "isError".

Code Review ca_upd20130920b.zip

The changes are good. The changes are probably safe enough to avoid separate regression testing. What do you think? If you agree, you can check in the changes now (and distribute).

Evgeny: please make sure you have applied this version of the code to your regression testing setup. In addition, you need to upload your final changes here so they can go though final code review.

The changes are good. The changes are probably safe enough to avoid separate regression testing. What do you think? If you agree, you can check in the changes now (and distribute).

Correct, MAJIC doesn't reach code using the new feature. ca_upd20130920b.zip was applied to rev 10384.

Evgeny: please merge your code with latest revision 10385. This is the version that needs final review and full regression testing.

Constantin Asofiei wrote:

Next step is to run regression testing; let me know if you encounter any problems.

I'm trying to run regression tests, but I think I have next issues:
1) Tests run forever for me, I've tried to run regression tests 4 times and 2 times it's just hang up, and other time I've stopped it after 5+ hours. How long is "normal" time for regression test running ?

I'm doing next things:
1)run_regression.sh configure
2)run_regression.sh build
3)run_regression.sh runtime-regression <<< myPass
4)run_regression.sh full-regression -Dsave.candidate=y -Dsrc.stable=20130923a -Dsrc.candidate=20130923b

Does phase 2 is necessary ? because I see, that 3 and 4 phase has building phase too?

Could someone send me or share running log for phase 3-4, because I want to know, how long tests could running ?

How long is "normal" time for regression test running ?

1. configure task - this one no more than 5 minutes
2. MAJIC conversion + build (the build task) - ~1 hour
3. runtime-regression - DB restore will take ~0.5-1 hour and after this two sets of testing are ran: CTRL-C will take ~1.5 hours, and the main part will take ~3.5 hours.
4. full-gression - this task doesn't support -D arguments, for the time being avoid using -D (it has a problem that for the first input it reads twice...).

Does phase 2 is necessary ? because I see, that 3 and 4 phase has building phase too?

Yes, as runtime-regression doesn't reconvert MAJIC. If you have conversion-related changes, then this step is needed.

Could someone send me or share running log for phase 3-4, because I want to know, how long tests could running ?

See above for the approximate times. Idea is, a full-regression will take ~6-7 hours, depending on machine load (and false negatives during tests).

I want to clarify something. Phase 2 (build) and 3 (runtime-regression) of your list above are needed. Phase 4 (full-regression) is the same thing as doing phases 2 and 3 together, so you don't have to run phase 4 if phases 2 and 3 are done separately. The "build" in phase 2 is really both converting and building Majic (the 4GL application). The "build" you see in phases 3 and 4 may be the P2J ant build process, which is different.

I've taken a look at the 20130926 and 20103929 harness results in your folder and I think we can say that testing has passed; 20130926 has only a GSO failure which is not in 20130929 and TC test suite passed in 20130929 (TC_JOB_002 failure is expected).

About the update: beside the methods_attributes.rules merge, the ErrorManager.java, XDocumentImpl.java and XmlFactory.java files don't contain real changes (when compared to evk_20130923a.zip version), just whitespace changes. Please keep only merged methods_attributes.rules file and replace the others with the evk_20130923a.zip archive.

At the commit message, please reference this task number, as in #1640.

Committed to bzr revision 10390