[
https://issues.apache.org/jira/browse/SQOOP-2006?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Qian Xu updated SQOOP-2006:
---------------------------
Description:
SQOOP-1995 fixed a NPE, which is adding a schema NPE check in 6 get/set FooData
methods. [~vybs] explained that the IDF class can be instantiated by
reflection, which will call the default no arg constructor, so that the
property schema will not be assigned expectedly.
IMO, although the patch can guard invalid access to a null schema, but as the
getter and setter methods have nothing to do with schema and there is no
comment left, it will be hard to understand why we have such code after couple
of months.
It seems a IDF class should have a not null schema the whole life time. I've
some options:
1. We check null schema where a NPE happens
2. Disallow reflection of IDF class or search all code occurrences to make sure
schema will be set properly (I think we already have test cases to guard use
cases)
3. Use some design by contract framework (https://github.com/nhatminhle/cofoja)
to define invariant, e.g. {{@Invariant(schema != null)}}.
was:
SQOOP-1995 fixed a NPE, which is adding a schema NPE check in 6 get/set FooData
methods. [~vybs] explained that the IDF class can be instantiated by
reflection, which will be constructed without setting a proper schema. The code
can guard invalid access to null schema. As the getter and setter methods do
not have anything to do with schema, and there is not a comment to explain
that, it will be hard to track the reason of having such code after couple of
month.
IMO, it seems a not null schema should be an invariant condition for a IDF
class. Either we check null schema where a NPE happens or use some design by
contract framework to define contracts.
I'd suggest move the schema validation to where a NPE happens instead of adding
it to all getter and setters.
> bad smell: NPE check in SQOOP-1995 is not obvious
> -------------------------------------------------
>
> Key: SQOOP-2006
> URL: https://issues.apache.org/jira/browse/SQOOP-2006
> Project: Sqoop
> Issue Type: Improvement
> Affects Versions: 1.99.5
> Reporter: Qian Xu
> Assignee: Qian Xu
> Priority: Minor
> Fix For: 1.99.5
>
>
> SQOOP-1995 fixed a NPE, which is adding a schema NPE check in 6 get/set
> FooData methods. [~vybs] explained that the IDF class can be instantiated by
> reflection, which will call the default no arg constructor, so that the
> property schema will not be assigned expectedly.
> IMO, although the patch can guard invalid access to a null schema, but as the
> getter and setter methods have nothing to do with schema and there is no
> comment left, it will be hard to understand why we have such code after
> couple of months.
> It seems a IDF class should have a not null schema the whole life time. I've
> some options:
> 1. We check null schema where a NPE happens
> 2. Disallow reflection of IDF class or search all code occurrences to make
> sure schema will be set properly (I think we already have test cases to guard
> use cases)
> 3. Use some design by contract framework
> (https://github.com/nhatminhle/cofoja) to define invariant, e.g.
> {{@Invariant(schema != null)}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
