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

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Won't Fix
    • Affects Version/s: 2.3.7
    • Fix Version/s: 2.3.10
    • Component/s: library
    • Labels:
      None
    • Sprint:
      SDK50: Sample App, DNS-SRV

      Attachments

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

        Activity

        Hide
        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.

        Show
        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.
        Hide
        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.

        Show
        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.
        Hide
        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

        Show
        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

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

            Dates

            • Created:
              Updated:
              Resolved:

              Gerrit Reviews

                PagerDuty

                Error rendering 'com.pagerduty.jira-server-plugin:PagerDuty'. Please contact your Jira administrators.