Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor "Options" API to be more in line with other bindings #121

Open
KrzysFR opened this issue May 22, 2023 · 0 comments
Open

Refactor "Options" API to be more in line with other bindings #121

KrzysFR opened this issue May 22, 2023 · 0 comments
Labels
api Issues or changes related to the client API enhancement

Comments

@KrzysFR
Copy link
Member

KrzysFR commented May 22, 2023

Looking at bindings for other languages, the transaction options are usually accessed via an "Options" property or field, so would look something like this in c#

tr.Options.Timeout = 10;
tr.Options.SetOption(FdbTransactionOption.AccessSystemKeys);

Currently, the SetOptions(...) methods are directly on the IFdbReadOnlyTransaction itself, and there is a set of extensions methods that target this interface to add the fluent versions of WithWriteAccessToSystemKeys(...) or WithNextWriteNoWriteConflictRange(..) etc..

These methods would be moved to a new IFdbTransactionOptions interface, and accessible via the tr.Options field.

Typical usage:

var results = await db.ReadWriteAsync(tr =>
{
     tr.Options.RetryLimit = 10;
     tr.Options.WithWriteAccessToSystemKeys();
     tr.Options.WithNextWriteNoWriteConflictRange();
     return tr.GetRange(....).Select(....).ToArrayAsync();
}, ct);

To keep an easy way to perform fluent configuration and query of a transaction (combined with the QueryAsync(...) helper, we would add a WithOptions(Action<IFdbTranscationOptions>) on the transaction itself, that would return the transaction instance. This would allow the caller to have the transaction handler expressed as a single expression, without the need to create a statement or block.

var res = tr.WithOptions(options => options.WithWriteAccessToSystemKeys().WithNextWriteNoWriteConflictRange())
                  .GetRange(....)
                  .Select(...)
                  .ToArrayAsync()

Pro:

  • look more like other bindings
  • less intellisense clutter on the transaction instance itself, with most of the options moved away into the tr.Options property
  • easier to mock or provide an alternative implementation (if not using the fdb_c client library).

Drawbacks:

  • is a breaking change for existing code that would call tr.WithXYZ(...) which need to change change to either tr.Options.WithXYZ(..) or use tr.WithOptions(options => options.WithXYZ()) instead.
@KrzysFR KrzysFR added enhancement api Issues or changes related to the client API labels May 22, 2023
KrzysFR added a commit that referenced this issue May 22, 2023
- Add Options field on transcations that return an IFdbTransactionOptions
- This interface has the SetOption(...) has well as most of the extension methods WithXYZ()
- Add missing values in enum FdbTransactionOption, up to API level 720
KrzysFR added a commit that referenced this issue May 22, 2023
- Similar to the work for transaction options, add an IFdbDatabaseOptions interface
- expose this via the "db.Options" field
- add extension methods on this new interface
- add missing FdbDatabaseOption values for up to API level 720
- Rename all "SetXYZ" into "WithXYZ" to be in line with the rest of the API
- Fix unit tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issues or changes related to the client API enhancement
Projects
None yet
Development

No branches or pull requests

1 participant