NFD » ndn-cxx

For this Issue we need to

1. Add tag deprecated to the functions:

   const RegisteredPrefixId*
   Face::registerPrefix(const Name& prefix,
                  const RegisterPrefixSuccessCallback& onSuccess,
                  const RegisterPrefixFailureCallback& onFailure,
                  const IdentityCertificate& certificate = IdentityCertificate(),
                  uint64_t flags = nfd::ROUTE_FLAG_CHILD_INHERIT);
   const RegisteredPrefixId*
   Face::registerPrefix(const Name& prefix,
                  const RegisterPrefixSuccessCallback& onSuccess,
                  const RegisterPrefixFailureCallback& onFailure,
                  const Name& identity,
                  uint64_t flags = nfd::ROUTE_FLAG_CHILD_INHERIT);
   const RegisteredPrefixId*
   Face::setInterestFilter(const InterestFilter& interestFilter,
                     const OnInterest& onInterest,
                     const RegisterPrefixSuccessCallback& onSuccess,
                     const RegisterPrefixFailureCallback& onFailure,
                     const IdentityCertificate& certificate = IdentityCertificate(),
                     uint64_t flags = nfd::ROUTE_FLAG_CHILD_INHERIT);
   const RegisteredPrefixId*
   Face::setInterestFilter(const InterestFilter& interestFilter,
                     const OnInterest& onInterest,
                     const RegisterPrefixFailureCallback& onFailure,
                     const IdentityCertificate& certificate = IdentityCertificate(),
                     uint64_t flags = nfd::ROUTE_FLAG_CHILD_INHERIT);
   const RegisteredPrefixId*
   Face::setInterestFilter(const InterestFilter& interestFilter,
                     const OnInterest& onInterest,
                     const RegisterPrefixSuccessCallback& onSuccess,
                     const RegisterPrefixFailureCallback& onFailure,
                     const Name& identity,
                     uint64_t flags = nfd::ROUTE_FLAG_CHILD_INHERIT);
   const RegisteredPrefixId*
   Face::setInterestFilter(const InterestFilter& interestFilter,
                     const OnInterest& onInterest,
                     const RegisterPrefixFailureCallback& onFailure,
                     const Name& identity,
                     uint64_t flags = nfd::ROUTE_FLAG_CHILD_INHERIT);
   

2. Create the following functions

   const RegisteredPrefixId*
   Face::registerPrefix(const Name& prefix,
                  const RegisterPrefixSuccessCallback& onSuccess,
                  const RegisterPrefixFailureCallback& onFailure,
                  const security::SigningInfo& signingInfo = security::SigningInfo(),,
                  uint64_t flags = nfd::ROUTE_FLAG_CHILD_INHERIT);
   const RegisteredPrefixId*
   Face::setInterestFilter(const InterestFilter& interestFilter,
                     const OnInterest& onInterest,
                     const RegisterPrefixSuccessCallback& onSuccess,
                     const RegisterPrefixFailureCallback& onFailure,
                     const security::SigningInfo& signingInfo = security::SigningInfo(),
                     uint64_t flags = nfd::ROUTE_FLAG_CHILD_INHERIT);
   const RegisteredPrefixId*
   Face::setInterestFilter(const InterestFilter& interestFilter,
                     const OnInterest& onInterest,
                     const RegisterPrefixFailureCallback& onFailure,
                     const security::SigningInfo& signingInfo = security::SigningInfo(),
                     uint64_t flags = nfd::ROUTE_FLAG_CHILD_INHERIT);
   

3. Add UT for the new functions

And this is it, right?

Hmm this cannot be done like this. Because we will be with 2 functions with the same interface.

  const RegisteredPrefixId*
  setInterestFilter(const InterestFilter& interestFilter,
                    const OnInterest& onInterest,
                    const RegisterPrefixSuccessCallback& onSuccess,
                    const RegisterPrefixFailureCallback& onFailure,
                    const IdentityCertificate& certificate = IdentityCertificate(),
                    uint64_t flags = nfd::ROUTE_FLAG_CHILD_INHERIT);
  const RegisteredPrefixId*
  setInterestFilter(const InterestFilter& interestFilter,
                    const OnInterest& onInterest,
                    const RegisterPrefixSuccessCallback& onSuccess,
                    const RegisterPrefixFailureCallback& onFailure,
                    const security::SigningInfo& signingInfo = security::SigningInfo(),
                    uint64_t flags = nfd::ROUTE_FLAG_CHILD_INHERIT);

One or the other need to stop having the default parameter.

I can see some options here:

1. To maintain backward compatibility the new functions will not have default. Eventually we can add default when we remove the old functions
2. Break backward compatibility and remove the default value of the old functions
3. Keep default in both, but change every place in the code where we call these functions using the default to start using the SigningInfo constructor as parameter

I neither confirm nor deny note-4. Please refer to issue description.

Reply to note-5: it's fine to remove the default of certificate on old API.

• old method: method(someArgs, const IdentityCertificate& certificate = IdentityCertificate(), moreArgs = moreDefaults)
• change to: method(someArgs, const IdentityCertificate& certificate, moreArgs = moreDefaults)
• new method: method(someArgs, const SigningInfo& signingInfo = SigningInfo(), moreArgs = moreDefaults)

This shouldn't break anything.

A default-constructed IdentityCertificate is an invalid certificate.

Its semantics, when used in registerPrefix or setInterestFilter, is signing with the default identity and certificate.

There are three ways to call the method:

• object.method(someArgs)
  • previously, this calls old method, and specifies signing with the default identity and certificate
  • after the change, this calls new method, and specifies signing with the default identity and certificate
  • there's no semantic change
• object.method(someArgs, IdentityCertificate())
  • previously, this calls old method
  • after the change, this still calls old method
• object.method(someArgs, IdentityCertificate(), moreArgs)
  • previously, this calls old method
  • after the change, this still calls old method

http://gerrit.named-data.net/2195

Changes:

1. Added the deprecated tag to the functions that should be deprecated
2. Added the new functions requested
3. Did not add UT because the UT present already tested the functions that I created because they used the default parameters

Path for NLSR http://gerrit.named-data.net/2206

Answering to note in gerrit saying "use a signing-helper"

Looking into the functions of the signing-helper the interface is pretty strange. Why do they all receive a Name? The functions structure doesn't look good. The functions should be overloads and receive different things.

Instead of :

SigningInfo
signingByIdentity(const Name& identity);

SigningInfo
signingByKey(const Name& keyName);

SigningInfo
signingByCertificate(const Name& certName);

Maybe we should have:

SigningInfo
signingBy(const Name& identity);

SigningInfo
signingBy(const KeyChain& key);

SigningInfo
signingBy(const IdentityCertificate& certificate);

Because with the functions as they are now what do we win by using them? Abstraction of an enum?

Reply to note-12:

This is irrelevant to this issue. Please discuss on #2922.

patch set 5 is the one with the uncommented define line