MohamedSaeed / xdocreport

Automatically exported from code.google.com/p/xdocreport
1 stars 0 forks source link

Can not generate fields.xml when there are bi-directionnal associations #448

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Create a series of classes with bidirectional onetoone or onetomany 
associations
2. call fieldsMetadata.load with such a class
3. wait. For a very very long time

What is the expected output? What do you see instead?
I expect to generate a fields.xml which can be used by openoffice macro as 
described here: 
https://code.google.com/p/xdocreport/wiki/ODTReportingQuickStart Instead 
program is stuck for a very long time. If i suspend program, i see a bunch of 
recursive calls and program is working on item

e.pData.employee.bonuses.employee.careerHistories.contract.careerHistory.employe
e.careerRanks.employee.children.employee.competenceMeasures.competenceMeasureTes
ts.competenceMeasure.employee.contracts.employee.cumuls.employee.current.functio
nalHead.employee.decorations.employee.detachments.employee.developmentCircles.em
ployee.diplomas.employee.formations.employee.handicaps.employee.heads.manager.ho
meHistory.employee.illnessContact1.contact1.illnessContact2.contact2.illnesses.e
mployee.leaves.employee.managedServices.service.childs.contactMaladie1.nextOfKin
.employee.promotionExams.employee.salaryRanks.employee.serviceMembers.service.cu
rrent.serviceHead.serviceHead.workAccidents.lesions.workAccident.employee.transp
ortation.employee.spp.employee.home.city

The program never stop (or i suppose it will when it will be out of memory). I 
stopped it after 10 minutes.

Solution: Code should not scan twice the same class.

What version of the product are you using? On what operating system?
1,0,4

Please provide any additional information below.

Original issue reported on code.google.com by tch...@gmail.com on 6 Oct 2014 at 8:32

GoogleCodeExporter commented 9 years ago
We know that fields metadata serializer have some limitation like 
bi-directionnal associations. 

But I think the model used inside template report docx/odt must not be the same 
than your DB model, because your template docx/odt will be complex.

@pascal : if you have time, could you see the problem?

Original comment by angelo.z...@gmail.com on 6 Oct 2014 at 8:38

GoogleCodeExporter commented 9 years ago
Will check, but IHMO bi directionnal associations are a bad idea...

Original comment by pascal.leclercq on 6 Oct 2014 at 8:39

GoogleCodeExporter commented 9 years ago
It a reality of application we report from, the DB model is bidirectionnal.
There are hundreds of fields in there. Recreating a second model just for the 
fields generation would be a very long and very error-prone process.

Some kind of custom filter on FreemarkerFieldsMetadata? Could be an option for 
us, as we could use the field annotations to prune inverse parts of 
bidirectionnal attributes.

public abstract class AbstractFieldsMetadataClassSerializer
    implements IFieldsMetadataClassSerializer
{
  private Predicate<PropertyDescriptor> filter;

  public void setFilter(Predicat<PropertyDescriptor> filter){
    this.filter=filter;
  }
//....
  private void process( FieldsMetadata fieldsMetadata, String key, Class<?> clazz, List<PropertyDescriptor> path, 
                          boolean isList )
        throws IntrospectionException
    {

        BeanInfo infos = Introspector.getBeanInfo( clazz );
        PropertyDescriptor[] descs = infos.getPropertyDescriptors();

        for ( PropertyDescriptor propertyDescriptor : descs )
        {

            // should not be transient and should have getter method
            if ( isTransient( propertyDescriptor, clazz ) || !haveGetterMethod( propertyDescriptor ) || !filter.test(propertyDescriptor))
                continue;

Original comment by tch...@gmail.com on 6 Oct 2014 at 8:52

GoogleCodeExporter commented 9 years ago
Hello,

i made a copy of AbstractFieldsMetadataClassSerializer and it's FreeMarker 
subclass in another package, made the setFilter static and it solved my problem 
+ will allow me some more flexibility in selecting available fields :)

Original comment by tch...@gmail.com on 6 Oct 2014 at 12:10

GoogleCodeExporter commented 9 years ago
Filter is a good idea.

@pascal, I think we should provide an interface IPropertyDescriptorFilter like 
this : 

------------------------------------------------
public interface IPropertyDescriptorFilter {
  IPropertyDescriptorFilter test(PropertyDescriptor descriptor);
}
------------------------------------------------

@tchize what do you think about that?

Original comment by angelo.z...@gmail.com on 6 Oct 2014 at 12:16

GoogleCodeExporter commented 9 years ago
Filter is a good idea.

@pascal, I think we should provide an interface IPropertyDescriptorFilter like 
this : 

------------------------------------------------
public interface IPropertyDescriptorFilter {
  boolean test(PropertyDescriptor descriptor);
}
------------------------------------------------

@tchize what do you think about that?

Original comment by angelo.z...@gmail.com on 6 Oct 2014 at 12:16

GoogleCodeExporter commented 9 years ago
Well, i used the java 8 java.util.function.Predicate here, but this pretty much 
leave to the same solution :)
The more i played with it today, the more i liked this idea, as i actually had 
more filtering to do than just avoid cycles. I also removed technical field and 
some collections that have no interest for final user :)

Original comment by tch...@gmail.com on 6 Oct 2014 at 1:17

GoogleCodeExporter commented 9 years ago
> Well, i used the java 8 java.util.function.Predicate here,
 dependencies just for predicate.
Yes I have seen that, but XDocReport support Java5, so I would like to avoid 
adding Java8 dependencies just for filter.

> The more i played with it today, the more i liked this idea, as i actually 
had more filtering to do than just avoid cycles. I also removed technical field 
and some collections that have no interest for final user :)

Cool! Ok I think we should add this filter to provide more flexibility.

Original comment by angelo.z...@gmail.com on 6 Oct 2014 at 1:24

GoogleCodeExporter commented 9 years ago
Hi all,

we can implement the 
public interface IPropertyDescriptorFilter {
  boolean test(PropertyDescriptor descriptor);

but before all I would like to have a better understanding.

Normally, we handle bidirectionnal relation like 
Personn <-> Adress.

as you can see here :
https://code.google.com/p/xdocreport/source/browse/template/fr.opensagres.xdocre
port.template/src/main/java/fr/opensagres/xdocreport/template/formatter/Abstract
FieldsMetadataClassSerializer.java#140

If you find a cycle, I suspect more complex relation graph such as :

Personn -> Address -> City -> Personn

which is not handled, thus we have a cycle.

We could store in a List all Classes that already has been processed this would 
prevent us for such cycle.

I think this would be easier for the end user not to have to create filters and 
handle PropertyDescriptor.

WDYT ?

Original comment by pascal.leclercq on 9 Oct 2014 at 1:16

GoogleCodeExporter commented 9 years ago
As you can see from original post, begin of line is:

e.pData.employee.......

in this case pData references Employee and Employee references pData, this is 
exactly 
employee<->pData and no, it was not filtered out it seems.

Also later on the same line:  "employee.children.employee"
It's also a simple Employee<=>Child relationShip

But i also have more complex relations:
Employee <==oneToMany==> Contracts
Employee <==oneToMany==> CareerHistory
CareerHistory <==OneToOne==> Contract

And a deeper one like Employe -> Service -> managerHistory -> manager (which is 
an Employee)

and even more problematic structure like

List<Contract> Employee.getContracts()
Contract[] Employee.getContractArray();
Contract Employee.getCurrent().getContract()

(with Current being a "view" of employee state) 

Filter interface, in my case, is more effective than automatic pruning of 
cycle, as automatic may do it on a way i don't like to do it, like forcing me 
to access contract from careerHistory entry instead of from Contract entry :)

Original comment by tch...@gmail.com on 9 Oct 2014 at 2:19

GoogleCodeExporter commented 9 years ago
So far I'm not able to reprocude.

I asked google to increase quota to allow you to upload file. This would help 
to to reprocude /write test.

thanks.

Original comment by pascal.leclercq on 9 Oct 2014 at 7:52

GoogleCodeExporter commented 9 years ago
I can not upload the whole project, for obvious reasons. And there is no XML 
file to upload, as the generation is looping indefinitely.

Original comment by tch...@gmail.com on 9 Oct 2014 at 8:29

GoogleCodeExporter commented 9 years ago
Commit 
8aea874157d78f541e05b9cd0fd351c025b38929

Original comment by pascal.leclercq on 13 Oct 2014 at 9:27