Couchbase Java Client

Allow persist argument to accept null/zero

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 1.1-dp3
  • Fix Version/s: 1.1-dp4
  • Component/s: library
  • Security Level: Public
  • Labels:
    None

Description

This makes it easier for people to turn persistence on/off.

Either provide a .ZERO constant, or check for those arguments to be null

i.e.


                    ft = cli.set(options.key,
                            options.expiry,
                            options.value,
                            null,
                            null);

Currently this throws an NPE

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Raghavan Srinivas added a comment -
How is this any different from set (key, expiry, value) which is already provided?

I think that is less confusing that set(key, expiry, value, null, null)

I agree that the NPE will need to be fixed.
Show
Raghavan Srinivas added a comment - How is this any different from set (key, expiry, value) which is already provided? I think that is less confusing that set(key, expiry, value, null, null) I agree that the NPE will need to be fixed.
Hide
Permalink
Mark Nunberg added a comment -
Because it requires the user to change the method call if they want to turn off persistence... functionally it is the same, but the former is exactly my use case.

So take this simple example:

PersistTo persist = .. // get global settings from app, if not available set to .ZERO
ReplicateTo replicate = ... // get global settings from app
OperationFuture ft = null;

// ....
// Right now, the user would need to do something like:

if (persist != null) {
 ft = cli.set(key, expiry, value);
} else {
 ft = cli.set(key, expiry, value, persist, replicate);
}

I've also just realized that the API provides no means by which to do replication without persistence.. allowing the 'extended' variant (k,e,v,p,r) gives the user pretty much everything he needs
Show
Mark Nunberg added a comment - Because it requires the user to change the method call if they want to turn off persistence... functionally it is the same, but the former is exactly my use case. So take this simple example: PersistTo persist = .. // get global settings from app, if not available set to .ZERO ReplicateTo replicate = ... // get global settings from app OperationFuture ft = null; // .... // Right now, the user would need to do something like: if (persist != null) {  ft = cli.set(key, expiry, value); } else {  ft = cli.set(key, expiry, value, persist, replicate); } I've also just realized that the API provides no means by which to do replication without persistence.. allowing the 'extended' variant (k,e,v,p,r) gives the user pretty much everything he needs
Hide
Permalink
Matt Ingenthron added a comment -
There should also be a variant that takes ReplicateTo without PersistTo.
Show
Matt Ingenthron added a comment - There should also be a variant that takes ReplicateTo without PersistTo.
Hide
Permalink
Michael Nitschinger added a comment -
See this changeset: http://review.couchbase.com/#/c/21421/
Show
Michael Nitschinger added a comment - See this changeset: http://review.couchbase.com/#/c/21421/

People

  • Assignee:
    Michael Nitschinger
    Reporter:
    Mark Nunberg
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: