> On May 7, 2015, 4:57 a.m., Amareshwari Sriramadasu wrote: > > lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommandsWithMissingWeight.java, > > line 110 > > <https://reviews.apache.org/r/33838/diff/2/?file=951313#file951313line110> > > > > Didnt this through an exception because of missing weight? > > Yash Sharma wrote: > No. > Infact the generated class for XFactTable used weight as primitive and > hence it gets the default of 0d. > > ```java > @XmlAttribute(name = "weight", required = true) > protected double weight; > ``` > > Amareshwari Sriramadasu wrote: > Then, I see making it "required" actually didnt solve the purpose. > Because missing value actually made it zero. > Inthat case I would say we might go with adding default as zero in xsd to > be clear, and allow only positive values for weight. Himanshu, thoughts? > > Himanshu Gahlaut wrote: > From perspective of domain objects, putting a checkArgument(weight > 0) > in AbstractCubeTable constructor shall help in making sure that domain object > CubeFactTable is constructed in valid state. > > For XSD and XFactTable, we can try using xs:restriction in XSD. Something > as below: > <xs:attribute name="weight"> > <xs:simpleType> > <xs:restriction base="xs:double"> > <xs:minExclusive value="0"></xs:minExclusive> > </xs:restriction> > </xs:simpleType> > </xs:attribute> > > and then using <xs:attribute ref="weight" use="required"/> in > XDimensionTablea nd XFactTable defintion in XSD. > > > For test case, one integration test case which makes sure that FactTable > creation gracefully fails when a fact is created with missing weight and one > unit test case which ensures that CubeFactTable instance creation throws > IllegalArgumentException when it is created with weight <= 0 shall give us > sufficient coverage.
@Himanshu: I am still able to pass my test scenario successfully with the xs:restriction implemented and missing weight attribute. The XFactTable weight still gets initialized with a default value of zero. Adding updated patch for review. - Yash ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33838/#review82792 ----------------------------------------------------------- On May 7, 2015, 4:42 p.m., Yash Sharma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33838/ > ----------------------------------------------------------- > > (Updated May 7, 2015, 4:42 p.m.) > > > Review request for lens. > > > Repository: lens > > > Description > ------- > > NPE when weight is not set in XFactTable object while creating fact table > > > Diffs > ----- > > lens-api/src/main/resources/cube-0.1.xsd 10b1dfa > > lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommandsWithMissingWeight.java > PRE-CREATION > lens-cli/src/test/resources/fact_without_weight.xml PRE-CREATION > > Diff: https://reviews.apache.org/r/33838/diff/ > > > Testing > ------- > > Yes. > TestCase: > mvn test -Dtest=TestLensFactCommandsWithMissingWeight#testFactCommands > > > Thanks, > > Yash Sharma > >
