You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Lavkesh Lahngir <la...@gmail.com> on 2016/06/27 17:57:46 UTC

Review Request 49275: [LENS-1205] Fix XMLToString

-----------------------------------------------------------
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

Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
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


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

Posted by Lavkesh Lahngir <la...@gmail.com>.

> On June 28, 2016, 6:26 a.m., Rajat Khandelwal wrote:
> > lens-api/src/main/java/org/apache/lens/api/ToXMLString.java, line 71
> > <https://reviews.apache.org/r/49275/diff/1/?file=1431213#file1431213line71>
> >
> >     If `tClass` is changed from `Class<T>` 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

Posted by Rajat Khandelwal <ra...@gmail.com>.

> On June 28, 2016, 11:56 a.m., Rajat Khandelwal wrote:
> > lens-api/src/main/java/org/apache/lens/api/ToXMLString.java, line 71
> > <https://reviews.apache.org/r/49275/diff/1/?file=1431213#file1431213line71>
> >
> >     If `tClass` is changed from `Class<T>` 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

Posted by Rajat Khandelwal <ra...@gmail.com>.

> On June 28, 2016, 11:56 a.m., Rajat Khandelwal wrote:
> > lens-api/src/main/java/org/apache/lens/api/ToXMLString.java, line 71
> > <https://reviews.apache.org/r/49275/diff/1/?file=1431213#file1431213line71>
> >
> >     If `tClass` is changed from `Class<T>` 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

Posted by Lavkesh Lahngir <la...@gmail.com>.

> On June 28, 2016, 6:26 a.m., Rajat Khandelwal wrote:
> > lens-api/src/main/java/org/apache/lens/api/ToXMLString.java, line 71
> > <https://reviews.apache.org/r/49275/diff/1/?file=1431213#file1431213line71>
> >
> >     If `tClass` is changed from `Class<T>` 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

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/49275/#comment205075>

    If `tClass` is changed from `Class<T>` 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)
<https://reviews.apache.org/r/49275/#comment205076>

    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
> 
>