search

osgi / bugzilla-archive

Archive of OSGi Alliance Specification Bugzilla bugs. The Specification Bugzilla system was decommissioned with the move to GitHub. The issues in this repository are imported from the Specification Bugzilla system for archival purposes.
0 stars 1 forks source link

[TR069ParameterValue]proposal of hiding constructor #1897

Closed bjhargrave closed 13 years ago

bjhargrave commented 13 years ago

Original bug ID: BZ#2029 From: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp> Reported version: R4 V4.3

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

This bug is related with the following bugs: https://www.osgi.org/members/bugzilla/show_bug.cgi?id=2005 https://www.osgi.org/members/bugzilla/show_bug.cgi?id=1938

While I was trying to fix Bug#2005, I encountered a problem regarding TR069ParamaterValue constructor.

[The latest spec] First of all, the javadoc of TR069ParameterValue(String value, String type) in Git repo says: ----quote begin---- Constructor of TR-069 Parameter Value.

@ param value value to be used. It can be either lexical or canonical representation as defined by XML Schema. @ param type data type defined in TR-069. @ throws IllegalArgumentException if value is {@ code null} or empty, if type is {@ code null}, if type is not any of defined types. ----quote end ----

On the other hand, the javadoc of getValue() says: ----quote begin---- Get the value this TR-069 Parameter value contains.

The value must be canonical representation as defined by XML Schema., e.g. upper-case Hex letters in hexBinary, and no leading zeroes in numeric values.

@ return value ----quote end ----

[The problem] While the constructor accepts lexical or canonical representation as a value, getValue() must return canonical representation. However, there is no guarantee that the value set in the constructor is in the appropriate format (lexical or cannonical representation) as the type. If not (for example, "abcde" as dateTime type), what to be returned by getValue() ? IMO, the current javadoc has inconsistency.

[My proposal] there are two ideas to go:

(Idea1)The constructor should not be defined in the spec. All functionality the current TR-069ParameterValue class is separated into two.

(Idea2)The constructor checks whether the value is comformant to the type.

I support Idea1.

Realizing Idea2 is troublesome to implement the class (perfect implementation of the check and the translation to the canonical representation is complicated). However, the reason why I had defined TR069ParameterValue class is

(DmtData data) or TR069ParameterValue.getTR069ParameterValueForList(DmtData data).

What do you think ?

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

Realizing Idea2 is troublesome to implement the class (perfect implementation of the check and the translation to the canonical representation is complicated). However, the reason why I had defined TR069ParameterValue class is

  • NOT realizing the translation from "lexical or canonical representation" to "canonical representation",
  • BUT getting a TR069ParameteValue object by calling TR069ParameterValue.getTR069ParameterValue

(DmtData data) or TR069ParameterValue.getTR069ParameterValueForList(DmtData data).

What do you think ?

I agree that TR069ParameterValue is important in the context of Dmt Admin i.e. the result of Dmt Admin data transformation. I think that Idea 1 (an additional utility class) will make the interfaces more complicated. I have another suggestion about the redesign:

  1. remove getTR069ParameterValueForList method
  2. remove getTR069ParameterValue method
  3. add a new constrictor TR069ParameterValue(DmtData[]), which will operate on list values
  4. add a new constructor TR069ParameterValue(DmtData), which will operate on single data
  5. hide TR069ParameterValue(String, String)
bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

I agree that TR069ParameterValue is important in the context of Dmt Admin i.e. the result of Dmt Admin data transformation. I think that Idea 1 (an additional utility class) will make the interfaces more complicated. I have another suggestion about the redesign:

  1. remove getTR069ParameterValueForList method
  2. remove getTR069ParameterValue method
  3. add a new constrictor TR069ParameterValue(DmtData[]), which will operate on list values
  4. add a new constructor TR069ParameterValue(DmtData), which will operate on single data
  5. hide TR069ParameterValue(String, String)

That is fine for me. How about others ?

bjhargrave commented 13 years ago

Comment author: @bjhargrave

That is fine for me. How about others ?

It seems to me that the util.tr069 package should not be coupled to the info.dmtree package. If there really is a coupling, then the tr069 package should be moved into the info.dmtree project as the info.dmtree.util.tr069 package.

A util package should be fairly standalone. As it sits now, the util.tr069 package is a bolt-on package for info.dmtree. The suggested changes here increase that coupling. This package should moved into the info.dmtree project.

bjhargrave commented 13 years ago

Comment author: Evgeni Grigorov <e.grigorov@prosyst.com>

That is fine for me. How about others ?

It seems to me that the util.tr069 package should not be coupled to the info.dmtree package. If there really is a coupling, then the tr069 package should be moved into the info.dmtree project as the info.dmtree.util.tr069 package.

A util package should be fairly standalone. As it sits now, the util.tr069 package is a bolt-on package for info.dmtree. The suggested changes here increase that coupling. This package should moved into the info.dmtree project.

Let me clarify a little bit, TR-069 Protocol Adapter works over Dmt Admin. It's a one layer above. So, we need an utility, which can convert Dmt Admin data to more convenient data for TR-069 Protocol Adapter. That means util.tr069 is closely coupled with Dmt Admin data.

On the other hand, Dmt Admin is core service for all Protocol Adapters, including SyncML, TR-069 etc. Dmt Admin is protocol independent. If we move util.tr069 to common info.dmtree project, we will link the common interfaces with protocol specific interfaces.

I am not the author of the utility classes, but I think that it's the general idea.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

I am not the author of the utility classes, but I think that it's the general idea.

That's right.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

We have discussed this on the unofficial conf call on June 8th 2011. Steffen agreed comment#1 and William will check it.

bjhargrave commented 13 years ago

Comment author: william.lupton@pace.com

I read the bug...

  1. canonical and lexical representation: "lexical" means "any representation of the value that is valid for the data type", whereas "canonical" means "the single preferred representation of the value"; so the canonical representation is always a valid lexical representation

see http://www.w3.org/TR/2004/REC-xmlschema-2-20041028/datatypes.html#typesystem:

for example, for unsignedInt, http://www.w3.org/TR/2004/REC-xmlschema-2-20041028/datatypes.html#unsignedInt:

so all that the "accept lexical in constructor, return canonical on get" rule means is that the constructor has to accept anything that is legal but doesn't have to remember how it was specified (e.g. whether there were leading zeros); this just means that the implementation can store the value using the appropriate native data type; surely it makes implementation EASIER, not HARDER?

  1. behaviour of string constructor: if there IS a string constructor I think it should validate both its arguments (value and type), so for example I would expect TR069ParameterValue("abcd", TR069_TYPE_UNSIGNED_INT) to throw an exception
  1. existence of string constructor? if the string constructor is replaced with DmtData / DmtData[] constructors, who will parse the TR-069-supplied string in the first place? it was mentioned that there was a static method to do that... but which one? personally I don't see what is wrong with a string constructor, but maybe i am missing the big picture of how the class is intended to be used
  1. perhaps someone could give a quick illustration of the proposed new class interface, and how it would be used when processing a SPV and a GPV?
bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Very sorry not to respond...

I read the bug...

  1. canonical and lexical representation: "lexical" means "any representation of the value that is valid for the data type", whereas "canonical" means "the single preferred representation of the value"; so the canonical representation is always a valid lexical representation see http://www.w3.org/TR/2004/REC-xmlschema-2-20041028/datatypes.html#typesystem:
    • [Definition:] A value space is the set of values for a given datatype. Each value in the value space of a datatype is denoted by one or more literals in its ·lexical space·.
    • [Definition:] A lexical space is the set of valid literals for a datatype.
    • [Definition:] A canonical lexical representation is a set of literals from among the valid set of literals for a datatype such that there is a one-to-one mapping between literals in the canonical lexical representation and values in the ·value space·. for example, for unsignedInt, http://www.w3.org/TR/2004/REC-xmlschema-2-20041028/datatypes.html#unsignedInt:
    • unsignedInt has a lexical representation consisting of a finite-length sequence of decimal digits (#x30-#x39). For example: 0, 1267896754, 100000.

    • The canonical representation for unsignedInt is defined by prohibiting certain options from the Lexical representation (§3.3.22.1). Specifically, leading zeroes are prohibited. so all that the "accept lexical in constructor, return canonical on get" rule means is that the constructor has to accept anything that is legal but doesn't have to remember how it was specified (e.g. whether there were leading zeros); this just means that the implementation can store the value using the appropriate native data type; surely it makes implementation EASIER, not HARDER?

Generally speaking, I agree.

  1. behaviour of string constructor: if there IS a string constructor I think it should validate both its arguments (value and type), so for example I would expect TR069ParameterValue("abcd", TR069_TYPE_UNSIGNED_INT) to throw an exception

If so, I agree. But current javadoc does not require it and impl does not work like that.

  1. existence of string constructor? if the string constructor is replaced with DmtData / DmtData[] constructors, who will parse the TR-069-supplied string in the first place? it was mentioned that there was a static method to do that... but which one? personally I don't see what is wrong with a string constructor, but maybe i am missing the big picture of how the class is intended to be used

What I wrote was correct.

If the constructor must check whether the specified value is comformant to "lexical representation defined by XML", it is hard and it is needed for our usecase (I think it is too much).

If the constructor must check whether the specified value is comformant to "lexical representation defined by TR-069", it is not and it is what current getDmtData() does. So leaving constructor public but modified is fine.

  1. perhaps someone could give a quick illustration of the proposed new class interface, and how it would be used when processing a SPV and a GPV?

If a TR-069PA receives GPV, getTR069ParameterValue(DmtData data) or getTR069ParameterValueForList() will be used. If a TR-069PA receives SPV, getDmtData(String value,String tr069Type,String valueCharsetName,String nodeUri,MetaNode metaNode) or getDmtDataForList() will be used.

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

I re-think this issue by myself again. Let me ask questions about dataTime data defined by TR-069.

=======================================

  1. Definition in TR-069.

--- BEGIN QUOTE ["Table 9 Data types" in TR-069 Amendment 3] --- dateTime The subset of the ISO 8601 date-time format defined by the SOAP dateTime type. All times MUST be expressed in UTC (Universal Coordinated Time) unless explicitly stated otherwise in the definition of a variable of this type. If absolute time is not available to the CPE, it SHOULD instead indicate the relative time since boot, where the boot time is assumed to be the beginning of the first day of January of year 1, or 0001-01-01T00:00:00. For example, 2 days, 3 hours, 4 minutes and 5 seconds since boot would be expressed as 0001-01-03T03:04:05. Relative time since boot MUST be expressed using an untimezoned representation. Any untimezoned value with a year value less than 1000 MUST be interpreted as a relative time since boot. If the time is unknown or not applicable, the following value representing “Unknown Time” MUST be used: 0001-01-01T00:00:00Z. Any dateTime value other than one expressing relative time since boot (as described above) MUST use timezoned representation (that is, it MUST include a timezone suffix). --- END QUOTE ["Table 9 Data types" in TR-069 Amendment 3] ---

  1. Definition in XML Schema. http://www.w3.org/TR/xmlschema-2/#dateTime

  2. Now, my questions are:

Q1. What is the difference between two ?

Q2. TR069ParameterValue.getDmtData()/getDmtDataForList() must support any lexical representation of dataTime defined by TR-069. Then, must those methods throws IllegalArgumentException if the argument is not compliant to any lexical representation of dataTime defined by TR-069? (or it does not because we assume that the value specified by SPV RPC must be lexical representation defined by TR-069.)

Let me discuss this today's REG F2F.

bjhargrave commented 13 years ago

Comment author: william.lupton@pace.com

Q1. What is the difference between two ?

TR-069 dateTime is the same as XML dateTime but there are are some additional usage conventions and constraints:

  1. specific value which represents unknown time
  2. convention for representing relative time since boot
  3. requirement for use of time zone for all absolute times

therefore i think that the only dateTime values that are valid XML values but invalid TR-069 values are absolute times (i.e. year >= 1000) that don't have time zones

Q2. TR069ParameterValue.getDmtData()/getDmtDataForList() must support any lexical representation of dataTime defined by TR-069. Then, must those methods throws IllegalArgumentException if the argument is not compliant to any lexical representation of dataTime defined by TR-069? (or it does not because we assume that the value specified by SPV RPC must be lexical representation defined by TR-069.)

i think so ... but let's discuss as you propose

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Q1. What is the difference between two ?

TR-069 dateTime is the same as XML dateTime but there are are some additional usage conventions and constraints:

  1. specific value which represents unknown time
  2. convention for representing relative time since boot
  3. requirement for use of time zone for all absolute times

therefore i think that the only dateTime values that are valid XML values but invalid TR-069 values are absolute times (i.e. year >= 1000) that don't have time zones

I got it.

Q2. TR069ParameterValue.getDmtData()/getDmtDataForList() must support any lexical representation of dataTime defined by TR-069. Then, must those methods throws IllegalArgumentException if the argument is not compliant to any lexical representation of dataTime defined by TR-069? (or it does not because we assume that the value specified by SPV RPC must be lexical representation defined by TR-069.)

i think so ... but let's discuss as you propose

OK and I understand the difference between two is not big by the answer to Q1.

Now I changed my opinion:

  1. The Constructor can be kept as public.
  2. The constructor will throw IllgalArgumentException if the specified value is not conformant to any lexical representation of the specified type defined by TR069.
  3. getDmtData() and getDmtDmtDataForList() will IllgalArgumentException if the specified value is not conformant to any lexical representation of the specified type defined by TR069.

In the impls of the constructor/getDmtData()/getDmtDmtDataForList() will share the logic for checking.

bjhargrave commented 13 years ago

Comment author: Kai Hackbarth <k.hackbarth@prosyst.com>

We have agreed to Ikuo's proposal in Comment No. 11

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

I'm going to update Javadoc as follows:

public TR069ParameterValue(String value, String type);

Constructor of TR-069 Parameter Value.

@ param value value to be used. It must be in the conformant format for the specified data type defined by TR-069. The conformant format is a subset of lexical representation of the data type as defined by XML Schema. @ param type TR-069 data type. @ throws IllegalArgumentException if value is {@ code null} or empty, if type is {@ code null}, if type is not any of defined types, or if value is not in the conformant format as the specified type defined by TR-069 and XML Schema.

public static DmtData getDmtData(final String value, final String tr069Type, final String valueCharsetName, final String nodeUri, final MetaNode metaNode) throws TR069MappingException, IllegalArgumentException, UnsupportedEncodingException;

@ param value value to be used. It must be in the conformant format for the specified data type defined by TR-069. The conformant format is a subset of lexical representation of the data type as defined by XML Schema. @ param tr069Type TR069 data type. @ throws java.lang.IllegalArgumentException if tr069Type is {@ code null}, if tr069Type is not any of defined types, if value is {@ code null}, or if value is not in the conformant format as the specified type defined by TR-069 and XML Schema., if tr069Type is not any of the defined ones, if nodeUri is not valid DMT Uri, or if metaNode is {@ code null}. --------------------------------------------- William or Evgeni, could you tell me if you have any beter expression idea ?
bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

Created attachment 188 Input/Output of TR069ParameterValue

I'm implementing TR069ParameterValue class of RI. For it, I had prepared the sheet which lists Expected results of canonical representation returned by getValue() method from lexical representation given for constructor, accoding to TR-069 and XML schema (Especially for William and Evgeni).

Attached file: TR069ParameterValue_20110710_1.xls (application/vnd.ms-excel, 22107 bytes) Description: Input/Output of TR069ParameterValue

bjhargrave commented 13 years ago

Comment author: @pkriens

I think this is stale

bjhargrave commented 13 years ago

Comment author: Ikuo Yamasaki <yamasaki.ikuo@lab.ntt.co.jp>

This is stale because the spec has been changed completely.