Uploaded image for project: 'Couchbase .NET client library'
  1. Couchbase .NET client library
  2. NCBC-1204

CouchbaseClientDefinition should use interface for Buckets property

    XMLWordPrintable

Details

    • Improvement
    • Status: Resolved
    • Major
    • Resolution: Won't Fix
    • 2.3.7
    • 2.3.10
    • library
    • None
    • SDK50: Sample App, DNS-SRV

    Attachments

      No reviews matched the request. Check your Options in the drop-down menu of this sections header.

      Activity

        jmorris Jeff Morris added a comment -

        Commit was reverted, it breaks the JSON configuration tests. The reason appears to be that Bind() does not honor binding to interface fields; you need a concrete implementation as a field value to map a JSON element.

        jmorris Jeff Morris added a comment - Commit was reverted, it breaks the JSON configuration tests. The reason appears to be that Bind() does not honor binding to interface fields; you need a concrete implementation as a field value to map a JSON element.

        After reviewing JsonConfigurationProvider and ConfigurationBinder, that are used when building settings from a JSON file, they do not support interface types. This is for all .NET framework interfaces (IList, IDictionary, etc) and custom application interfaces, for example in this case IBucketConfiguration. While it would be possible to build our own CouchbaseConfigurationBinder that could be provided with a mapping of interfaces to class implementations, it does feel like a lot of work for not so much reward.

        What do you think Jeff Morris?

        PS I've provided a Gerrit commit to add an explicit test that demonstrates our current implementation works as expected. If the Buckets property on CouchbaseClientDefinition was changed to List<IBucketDefinition>, the test will fail.

        mike.goldsmith Michael Goldsmith added a comment - After reviewing JsonConfigurationProvider and ConfigurationBinder, that are used when building settings from a JSON file, they do not support interface types. This is for all .NET framework interfaces (IList, IDictionary, etc) and custom application interfaces, for example in this case IBucketConfiguration. While it would be possible to build our own CouchbaseConfigurationBinder that could be provided with a mapping of interfaces to class implementations, it does feel like a lot of work for not so much reward. What do you think Jeff Morris ? PS I've provided a Gerrit commit to add an explicit test that demonstrates our current implementation works as expected. If the Buckets property on CouchbaseClientDefinition was changed to List<IBucketDefinition>, the test will fail.
        jmorris Jeff Morris added a comment -

        MIke -

        I agree seems like a lot of work for little effort. If it comes up again, we can always take the more difficult approach if it make sense.

        Thanks for adding the test!

        -Jeff

        jmorris Jeff Morris added a comment - MIke - I agree seems like a lot of work for little effort. If it comes up again, we can always take the more difficult approach if it make sense. Thanks for adding the test! -Jeff

        People

          jmorris Jeff Morris
          jmorris Jeff Morris
          Votes:
          0 Vote for this issue
          Watchers:
          2 Start watching this issue

          Dates

            Created:
            Updated:
            Resolved:

            Gerrit Reviews

              PagerDuty