-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Remove redundant methods from DocumentMapper #69803
Remove redundant methods from DocumentMapper #69803
Conversation
DocumentMapper exposes root() and meta() methods, which can be accessed through the mapping() method which exposes the entire Mapping instance. This commit removes such redundant methods in favour of accessing mapping and retrieving root and meta from them. Additionally, access to Mapping's members is made consistent through getters rather than package private fields in some cases and getters is some other case.
DocumentMapper exposes root() and meta() methods, which can be accessed through the mapping() method which exposes the entire Mapping instance. This commit removes such redundant methods in favour of accessing mapping and retrieving root and meta from them. Additionally, access to Mapping's members is made consistent through getters rather than package private fields in some cases and getters is some other case.
Pinging @elastic/es-search (Team:Search) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I think we can rationalise access to metadata mappers some more as well, but that can wait for later.
.filter(field -> deleteTombstoneMetadataFields.contains(field.name())).toArray(MetadataFieldMapper[]::new); | ||
final Collection<String> noopTombstoneMetadataFields = Arrays.asList( | ||
VersionFieldMapper.NAME, SeqNoFieldMapper.NAME, SeqNoFieldMapper.PRIMARY_TERM_NAME, SeqNoFieldMapper.TOMBSTONE_NAME); | ||
this.noopTombstoneMetadataFieldMappers = Stream.of(mapping.metadataMappers) | ||
this.noopTombstoneMetadataFieldMappers = Stream.of(mapping.getSortedMetadataMappers()) | ||
.filter(field -> noopTombstoneMetadataFields.contains(field.name())).toArray(MetadataFieldMapper[]::new); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure all this can just be replaced with a static set of an IdFieldMapper, SeqNoFieldMapper and VersionFieldMapper, but that can be done in a followup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes this is all a bit confusing, it seems very delicate too as in some places we need them sorted, in others we need the map by class, in others we retrieve them by name. I also wonder if we can further clean up DocumentMapper around retrieving metadata mappers.
DocumentMapper exposes root() and meta() methods, which can be accessed through the mapping() method which exposes the entire Mapping instance. This commit removes such redundant methods in favour of accessing mapping and retrieving root and meta from them. Additionally, access to Mapping's members is made consistent through getters rather than package private fields in some cases and getters is some other case.
DocumentMapper exposes root() and meta() methods, which can be accessed through the mapping() method which exposes the entire Mapping instance. This commit removes such redundant methods in favour of accessing mapping and retrieving root and meta from them. Additionally, access to Mapping's members is made consistent through getters rather than package private fields in some cases and getters is some other case.
DocumentMapper exposes root() and meta() methods, which can be accessed through the mapping() method that exposes the entire Mapping instance.
This commit removes such redundant methods in favour of accessing mapping and retrieving root and meta from them. Additionally, access to Mapping's members is made consistent through getters rather than package private fields in some cases and getters is some other case.