Skip to content

Scripted fields not mapped correctly during reading #3022

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
llosimura opened this issue Dec 12, 2024 · 1 comment · Fixed by #3023
Closed

Scripted fields not mapped correctly during reading #3022

llosimura opened this issue Dec 12, 2024 · 1 comment · Fixed by #3023
Labels
type: bug A general bug

Comments

@llosimura
Copy link
Contributor

llosimura commented Dec 12, 2024

I just noticed that if we set a custom name for a scripted field, it won't be mapped correctly to the entity in question while reading it.

Example:

@Document("my-test")
class ScriptFieldProperty {
     ...
     @ScriptedField(name = "scripted-field-property")
     String scriptedFieldProperty;
}

I did some investigation and noticed that when we moved the populateScriptFields method from using ReflectionUtils to PersitenceEntity on 9d57410, this condition:

 if (property.isAnnotationPresent(ScriptedField.class) && fields.containsKey(property.getName())) {

will never be true for custom names,since property#getNames is unaware of the ScriptedField annotation. In the previous example, it will always return scriptedFieldProperty instead of the expected scripted-field-property

I can see two ways of solving the issue.

  1. Remove the second part of the conditional and do the check internally. The name should come first from the annotation and only fall back to the property if it is not set. (Note this is already done before setting the value)
  2. Make SimpleElasticsearchPersistentProperty aware of the ScriptedField annotation. By adding a condition to check it in SimpleElasticsearchPersistentProperty#getAnnotatedFieldName, we can also ensure that the property has the correct name when populating the scripted fields.

I'm unsure of any other valid solution and would love some input. I have both solutions implemented locally and can create a PR if needed

Update: created a PR with the changes here #3023

@llosimura llosimura changed the title Scripted Fields Not Using correct Naming durin Scripted fields not mapped correctly during reading Dec 12, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 12, 2024
@sothawo
Copy link
Collaborator

sothawo commented Dec 13, 2024

thank you for finding this. I will have a look at the weekend

@sothawo sothawo added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 13, 2024
sothawo pushed a commit that referenced this issue Dec 14, 2024

Unverified

This user has not yet uploaded their public signing key.
Original Pull Request: #3023
Closes: #3022

(cherry picked from commit 944e7e8)
sothawo pushed a commit that referenced this issue Dec 14, 2024

Unverified

This user has not yet uploaded their public signing key.
Original Pull Request: #3023
Closes: #3022

(cherry picked from commit 944e7e8)
(cherry picked from commit 5d52918)
@sothawo sothawo added this to the 5.5 M1 (2025.0.0) milestone Dec 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
3 participants