Skip to content

Default attribute capabilities #707

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

Closed
bart-degreed opened this issue Mar 10, 2020 · 1 comment · Fixed by #721
Closed

Default attribute capabilities #707

bart-degreed opened this issue Mar 10, 2020 · 1 comment · Fixed by #721

Comments

@bart-degreed
Copy link
Contributor

bart-degreed commented Mar 10, 2020

What my team loves about JADNC is that resource properties require explicit annotation to expose them (opt-in). This guards against someone adding a new entity property that gets exposed unintentionally, requiring a breaking change to fix that.

But the capabilities on such annotated properties are opt-out. Adding [Attr] without arguments makes them mutable, filterable and sortable. We'd like to require explicitly stating that as well, to prevent users searching on columns that are not indexed in the database, resulting in a full table scan that kills performance on large tables.

If changing the defaults for parameterless AttrAttribute is too much of a breaking change, we can add a fallback setting to IJonApiOptions that controls the parameterless behavior. For example:

options => options.DefaultAttributeCapabilities = AttributeCapabilities.None

Where AttributeCapabilities is defined as:

[Flags]
public enum AttributeCapabilities
{
  None = 0,
  AllowMutate = 1,
  AllowFilter = 2,
  AllowSort = 4,
  All = AllowMutate | AllowFilter | AllowSort
}

And then we can replace the existing constructor overload that takes multiple booleans with one that accepts these flags. For example:

public class Article : Identifyable
{
    [Attr(AttributeCapabilities.AllowFilter | AttributeCapabilities.AllowMutate)]
    public string Description { get; set; }
}

Which would improve code readability over a line like:

Attr(true, true, false)]
@bart-degreed
Copy link
Contributor Author

Discussed my proposal offline with @maurei. We concluded this is a good idea, but need to include the fallback setting in options to preserve backwards compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant