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

Create extension methods for Facet field types #407

Closed
NightOwl888 opened this issue Feb 4, 2021 · 2 comments · Fixed by #468
Closed

Create extension methods for Facet field types #407

NightOwl888 opened this issue Feb 4, 2021 · 2 comments · Fixed by #468
Assignees
Milestone

Comments

@NightOwl888
Copy link
Contributor

NightOwl888 commented Feb 4, 2021

To make adding fields more discoverable via Intellisense, we have already created extension methods for "normal" field types here. However, we still need extension methods for facet field types defined within Lucene.Net.Facet to make the API consistent.

Proposed API

namespace Lucene.Net.Documents.Extensions
{
    public static class DocumentExtensions // Check to ensure the name doesn't collide with that in the Lucene.Net assembly, rename to DocumentFacetExtensions if necessary
    {
        public static FacetField AddFacetField(this document, string dim, params string[] path);
        public static SortedSetDocValuesFacetField AddSortedSetDocValuesFacetField(this document, string dim, string label);
        public static AssociationFacetField AddAssociationFacetField(this document, BytesRef assoc, string dim, params string[] path);
        public static Int32AssociationFacetField AddInt32AssociationFacetField(this document, int assoc, string dim, params string[] path);
        public static SingleAssociationFacetField AddSingleAssociationFacetField(this document, float assoc, string dim, params string[] path);
    }
}

Unit Tests

In addition, there are currently no tests for these extension methods, as the tests are using the existing Java API syntax. We should set up some basic unit tests to ensure the correct field type is added to a Document with the correct values and returned from the extension method correctly (both for the original extension methods and for the new facet extension methods).

@NightOwl888 NightOwl888 added up-for-grabs This issue is open to be worked on by anyone is:feature design good-first-issue Good for newcomers is:enhancement New feature or request pri:normal labels Feb 4, 2021
@NightOwl888 NightOwl888 added this to the 4.8.0 milestone Feb 4, 2021
@NightOwl888 NightOwl888 removed good-first-issue Good for newcomers up-for-grabs This issue is open to be worked on by anyone labels Apr 16, 2021
@NightOwl888 NightOwl888 self-assigned this Apr 16, 2021
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 16, 2021
…xtensions namespace. Added tests for DocumentExtensions in Lucene.Net.Tests._J-S, Lucene.Net.Tests.ICU and Lucene.Net.Tests.Facet. Added guard clauses and updated documentation of Document extension methods and some related fields. Closes apache#407.
NightOwl888 added a commit to NightOwl888/lucenenet that referenced this issue Apr 16, 2021
…xtensions namespace. Added tests for DocumentExtensions in Lucene.Net.Tests._J-S, Lucene.Net.Tests.ICU and Lucene.Net.Tests.Facet. Added guard clauses and updated documentation of Document extension methods and some related fields. Closes apache#407.
@NightOwl888
Copy link
Contributor Author

It turns out these extension methods already existed, but they were in in the Lucene.Net.Documents namespace so I missed them in Intellisense. So, #468 moves them to the correct namespace Lucene.Net.Documents.Extensions and adds tests to close this issue.

NightOwl888 added a commit that referenced this issue Apr 16, 2021
…xtensions namespace. Added tests for DocumentExtensions in Lucene.Net.Tests._J-S, Lucene.Net.Tests.ICU and Lucene.Net.Tests.Facet. Added guard clauses and updated documentation of Document extension methods and some related fields. Closes #407.
@rclabo
Copy link
Contributor

rclabo commented Aug 12, 2021

Just bumped into this breaking change but it makes perfect sense these were moved into an Lucene.Net.Documents.Extension.DocumentExtensions class. And thankfully in Visual Studio, ctl + . makes it so easy to find the new namespace that needs added to a project to access these extension methods.

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

Successfully merging a pull request may close this issue.

2 participants