Review Request 49275: [LENS-1205] Fix XMLToString

2016-06-27 Thread Lavkesh Lahngir 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49275/
---

Review request for lens.


Bugs: LENS-1205
https://issues.apache.org/jira/browse/LENS-1205


Repository: lens


Description
---

Classes generated from xsd do not contain the @XMLRootElement. We need to Pass 
a JAXBElement object while calling marshal() method. Also at the time 
unmarshal(), it returns the JAXBElement.


Diffs
-

  lens-api/src/main/java/org/apache/lens/api/ToXMLString.java fca56a8 
  
lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.xml
 ef7d0c7 
  
lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.yaml
 567d8cf 
  lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 
e866eb3 

Diff: https://reviews.apache.org/r/49275/diff/


Testing
---


Thanks,

Lavkesh Lahngir

Re: Review Request 49275: [LENS-1205] Fix XMLToString

2016-06-27 Thread Rajat Khandelwal 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49275/#review139735
---




lens-api/src/main/java/org/apache/lens/api/ToXMLString.java (line 61)


If `tClass` is changed from `Class` to just `Class`, then the caller has 
no idea what to pass there. We can't expect the callers to pass the correct 
`ObjectFactory` class. Can we revert this and incorporate the logic of 
picking/not picking object factory or package name here?



lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 
(line 430)


use the function `org.apache.lens.api.scheduler.ObjectFactory#createJob`.


- Rajat Khandelwal


On June 27, 2016, 11:27 p.m., Lavkesh Lahngir wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49275/
> ---
> 
> (Updated June 27, 2016, 11:27 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1205
> https://issues.apache.org/jira/browse/LENS-1205
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> Classes generated from xsd do not contain the @XMLRootElement. We need to 
> Pass a JAXBElement object while calling marshal() method. Also at the time 
> unmarshal(), it returns the JAXBElement.
> 
> 
> Diffs
> -
> 
>   lens-api/src/main/java/org/apache/lens/api/ToXMLString.java fca56a8 
>   
> lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.xml
>  ef7d0c7 
>   
> lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.yaml
>  567d8cf 
>   
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 
> e866eb3 
> 
> Diff: https://reviews.apache.org/r/49275/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>

Re: Review Request 49275: [LENS-1205] Fix XMLToString

2016-06-28 Thread Lavkesh Lahngir 


> On June 28, 2016, 6:26 a.m., Rajat Khandelwal wrote:
> > lens-api/src/main/java/org/apache/lens/api/ToXMLString.java, line 71
> > 
> >
> > If `tClass` is changed from `Class` to just `Class`, then the caller 
> > has no idea what to pass there. We can't expect the callers to pass the 
> > correct `ObjectFactory` class. Can we revert this and incorporate the logic 
> > of picking/not picking object factory or package name here?

The package logic fails with lensseesionhandle. It says "package" doesnt 
contain ObjectFactory.class or jaxb.index.


- Lavkesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49275/#review139735
---


On June 27, 2016, 5:57 p.m., Lavkesh Lahngir wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49275/
> ---
> 
> (Updated June 27, 2016, 5:57 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1205
> https://issues.apache.org/jira/browse/LENS-1205
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> Classes generated from xsd do not contain the @XMLRootElement. We need to 
> Pass a JAXBElement object while calling marshal() method. Also at the time 
> unmarshal(), it returns the JAXBElement.
> 
> 
> Diffs
> -
> 
>   lens-api/src/main/java/org/apache/lens/api/ToXMLString.java fca56a8 
>   
> lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.xml
>  ef7d0c7 
>   
> lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.yaml
>  567d8cf 
>   
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 
> e866eb3 
> 
> Diff: https://reviews.apache.org/r/49275/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>

Re: Review Request 49275: [LENS-1205] Fix XMLToString

2016-06-28 Thread Rajat Khandelwal 


> On June 28, 2016, 11:56 a.m., Rajat Khandelwal wrote:
> > lens-api/src/main/java/org/apache/lens/api/ToXMLString.java, line 71
> > 
> >
> > If `tClass` is changed from `Class` to just `Class`, then the caller 
> > has no idea what to pass there. We can't expect the callers to pass the 
> > correct `ObjectFactory` class. Can we revert this and incorporate the logic 
> > of picking/not picking object factory or package name here?
> 
> Lavkesh Lahngir wrote:
> The package logic fails with lensseesionhandle. It says "package" doesnt 
> contain ObjectFactory.class or jaxb.index.

Either we'll have to add if/else logic for this, or we should add jaxb.index 
files in all such packages. Either way, the function `valueOf` has to be 
generic and needs to work for all serializable classes.


- Rajat


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49275/#review139735
---


On June 27, 2016, 11:27 p.m., Lavkesh Lahngir wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49275/
> ---
> 
> (Updated June 27, 2016, 11:27 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1205
> https://issues.apache.org/jira/browse/LENS-1205
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> Classes generated from xsd do not contain the @XMLRootElement. We need to 
> Pass a JAXBElement object while calling marshal() method. Also at the time 
> unmarshal(), it returns the JAXBElement.
> 
> 
> Diffs
> -
> 
>   lens-api/src/main/java/org/apache/lens/api/ToXMLString.java fca56a8 
>   
> lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.xml
>  ef7d0c7 
>   
> lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.yaml
>  567d8cf 
>   
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 
> e866eb3 
> 
> Diff: https://reviews.apache.org/r/49275/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>

Re: Review Request 49275: [LENS-1205] Fix XMLToString

2016-06-28 Thread Rajat Khandelwal 


> On June 28, 2016, 11:56 a.m., Rajat Khandelwal wrote:
> > lens-api/src/main/java/org/apache/lens/api/ToXMLString.java, line 71
> > 
> >
> > If `tClass` is changed from `Class` to just `Class`, then the caller 
> > has no idea what to pass there. We can't expect the callers to pass the 
> > correct `ObjectFactory` class. Can we revert this and incorporate the logic 
> > of picking/not picking object factory or package name here?
> 
> Lavkesh Lahngir wrote:
> The package logic fails with lensseesionhandle. It says "package" doesnt 
> contain ObjectFactory.class or jaxb.index.
> 
> Rajat Khandelwal wrote:
> Either we'll have to add if/else logic for this, or we should add 
> jaxb.index files in all such packages. Either way, the function `valueOf` has 
> to be generic and needs to work for all serializable classes.

Or, you can check if the class has `XmlRoot` annotation. Use class if 
annotation is there, use package if it's not there.


- Rajat


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49275/#review139735
---


On June 27, 2016, 11:27 p.m., Lavkesh Lahngir wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49275/
> ---
> 
> (Updated June 27, 2016, 11:27 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1205
> https://issues.apache.org/jira/browse/LENS-1205
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> Classes generated from xsd do not contain the @XMLRootElement. We need to 
> Pass a JAXBElement object while calling marshal() method. Also at the time 
> unmarshal(), it returns the JAXBElement.
> 
> 
> Diffs
> -
> 
>   lens-api/src/main/java/org/apache/lens/api/ToXMLString.java fca56a8 
>   
> lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.xml
>  ef7d0c7 
>   
> lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.yaml
>  567d8cf 
>   
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 
> e866eb3 
> 
> Diff: https://reviews.apache.org/r/49275/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>

Re: Review Request 49275: [LENS-1205] Fix XMLToString

2016-06-28 Thread Lavkesh Lahngir 


> On June 28, 2016, 6:26 a.m., Rajat Khandelwal wrote:
> > lens-api/src/main/java/org/apache/lens/api/ToXMLString.java, line 71
> > 
> >
> > If `tClass` is changed from `Class` to just `Class`, then the caller 
> > has no idea what to pass there. We can't expect the callers to pass the 
> > correct `ObjectFactory` class. Can we revert this and incorporate the logic 
> > of picking/not picking object factory or package name here?
> 
> Lavkesh Lahngir wrote:
> The package logic fails with lensseesionhandle. It says "package" doesnt 
> contain ObjectFactory.class or jaxb.index.
> 
> Rajat Khandelwal wrote:
> Either we'll have to add if/else logic for this, or we should add 
> jaxb.index files in all such packages. Either way, the function `valueOf` has 
> to be generic and needs to work for all serializable classes.
> 
> Rajat Khandelwal wrote:
> Or, you can check if the class has `XmlRoot` annotation. Use class if 
> annotation is there, use package if it's not there.

Okay. Right now it works for both ObjectFactory and XMLRootElement classes. 
When marshalling the XML, if JAXBElement is used, then we should use the same 
thing while unmarshalling.


- Lavkesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49275/#review139735
---


On June 27, 2016, 5:57 p.m., Lavkesh Lahngir wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49275/
> ---
> 
> (Updated June 27, 2016, 5:57 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-1205
> https://issues.apache.org/jira/browse/LENS-1205
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> Classes generated from xsd do not contain the @XMLRootElement. We need to 
> Pass a JAXBElement object while calling marshal() method. Also at the time 
> unmarshal(), it returns the JAXBElement.
> 
> 
> Diffs
> -
> 
>   lens-api/src/main/java/org/apache/lens/api/ToXMLString.java fca56a8 
>   
> lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.xml
>  ef7d0c7 
>   
> lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.yaml
>  567d8cf 
>   
> lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 
> e866eb3 
> 
> Diff: https://reviews.apache.org/r/49275/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Lavkesh Lahngir
> 
>

Re: Review Request 49275: [LENS-1205] Fix XMLToString

2016-06-28 Thread Lavkesh Lahngir 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49275/
---

(Updated June 28, 2016, 3:01 p.m.)


Review request for lens.


Changes
---

Includes bug fixes in the SchedulerDAO


Bugs: LENS-1205
https://issues.apache.org/jira/browse/LENS-1205


Repository: lens


Description
---

Classes generated from xsd do not contain the @XMLRootElement. We need to Pass 
a JAXBElement object while calling marshal() method. Also at the time 
unmarshal(), it returns the JAXBElement.


Diffs (updated)
-

  lens-api/src/main/java/org/apache/lens/api/LensSessionHandle.java 74725e3 
  lens-api/src/main/java/org/apache/lens/api/ToXMLString.java fca56a8 
  
lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.xml
 ef7d0c7 
  
lens-api/src/test/resources/toString/org.apache.lens.api.query.SchedulerJobHandle.yaml
 567d8cf 
  lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerDAO.java 
e866eb3 

Diff: https://reviews.apache.org/r/49275/diff/


Testing
---


Thanks,

Lavkesh Lahngir