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

Improved: EntityUtil getProperty Methods dont use entity (OFBIZ-12815) #635

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

thahn27
Copy link

@thahn27 thahn27 commented May 8, 2023

The getProperty methods in EntityUtilProperties don't use entity at all. All of the getProperty methods simply lead to UtilProperties and therefore no configure during runtime is possible.

Improved: New methods have been written so the entity usage is now functional.

The getProperty methods in EntityUtilProperties don't use entity at all.
All of the getProperty methods simply lead to UtilProperties and
therefore no configure during runtime is possible. New methods have been
written so the entity usage is now functional.
@sonarcloud
Copy link

sonarcloud bot commented May 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@JacquesLeRoux
Copy link
Contributor

This sounds good to me.

Should we not deprecate the previous getPropertyNumber(String resource, String name, double defaultValue) until getPropertyAsBigDecimal(String resource, String name, BigDecimal defaultNumber) ?

@thahn27
Copy link
Author

thahn27 commented May 8, 2023

Hey Jacques,

I will provide another commit in which i adjust the switch statement mentioned by Gil and also deprecate the previous ones.

@JacquesLeRoux
Copy link
Contributor

JacquesLeRoux commented May 9, 2023

Hi @thahn27,

I checked the usage in Java and Groovy code of the previous methods I talked about above. Most of them are not used at all OOTB.
getPropertyAsBoolean and getPropertyAsBigDecimal are both used twice in Java, getPropertyAsInteger is used 21 times in Java and 6 in Groovy.

It could be that OFBiz users are using them more. But it's really easy to replace them by UtilProperties ones or the new EntityUtilProperties ones. So maybe we could remove them all together OOTB. And replace those used by calls to the new ones you introduce. I'll start a discussion on dev ML about that.

@JacquesLeRoux
Copy link
Contributor

I send an email to dev ML 1 hour ago but it does not appear yet. I wonder why, but let's wait

@@ -123,6 +123,117 @@ public static String getPropertyValueFromDelegatorName(String resource, String n
}
}

public static <T> Object getPropertyValue(String resource, String name, Object defaultValue, Delegator delegator, Class<T> clazz) {
Map<String, String> propMap = getSystemPropertyValue(resource, name, delegator);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use ObjectType.simpleTypeConvert method ?

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