Skip to content

InMemPagedTRS handles concurrency. #855

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

Merged
berezovskyi merged 3 commits into from
Dec 13, 2025
Merged

InMemPagedTRS handles concurrency. #855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
205e765
InMemPagedTRS handles concurrency.
Jad-el-khoury Dec 5, 2025
117ac80
bug fix in trs-client tests.
Jad-el-khoury Dec 5, 2025
8c96f55
Merge branch 'master' into trs_concurrency
berezovskyi Dec 13, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ This release does not contain new features.
- 🧨 Migrated to Jena 5. JDK 21 is the baseline requirement. JSON-LD 1.1 is used instead of JSON-LD 1.0.
- InputStream is now the preferred interface for initializing OslcQueryResult
- RootServicesHelper can be initialized using an InputStream
- InMemPagedTRS handles concurrency.

### Deprecated

Expand Down
136 changes: 69 additions & 67 deletions core/oslc-trs/src/main/java/org/eclipse/lyo/core/trs/Base.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.net.URI;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;

import org.eclipse.lyo.oslc4j.core.annotation.OslcDescription;
import org.eclipse.lyo.oslc4j.core.annotation.OslcHidden;
Expand All @@ -47,14 +48,14 @@
*
* {@code
* <https://.../baseResources>
* a ldp:Container;
* trs:cutoffEvent <urn:urn-3:cm1.example.com:2010-10-27T17:39:31.000Z:101> ;
* rdfs:member <http://cm1.example.com/bugs/1> ;
* rdfs:member <http://cm1.example.com/bugs/2> ;
* rdfs:member <http://cm1.example.com/bugs/3> ;
* ...
* rdfs:member <http://cm1.example.com/bugs/199> ;
* rdfs:member <http://cm1.example.com/bugs/200> .
* a ldp:Container;
* trs:cutoffEvent <urn:urn-3:cm1.example.com:2010-10-27T17:39:31.000Z:101> ;
* rdfs:member <http://cm1.example.com/bugs/1> ;
* rdfs:member <http://cm1.example.com/bugs/2> ;
* rdfs:member <http://cm1.example.com/bugs/3> ;
* ...
* rdfs:member <http://cm1.example.com/bugs/199> ;
* rdfs:member <http://cm1.example.com/bugs/200> .
* }
* </pre>
*
Expand All @@ -70,9 +71,9 @@
* <pre>
* {@code
* <https://.../baseResources/page1>
* a ldp:Page;
* ldp:pageOf <https://.../baseResource>;
* ldp:nextPage <https://../baseResources/page2> .
* a ldp:Page;
* ldp:pageOf <https://.../baseResource>;
* ldp:nextPage <https://../baseResources/page2> .
* }
* </pre>
*
Expand Down Expand Up @@ -130,65 +131,66 @@
@OslcName(LDP_TERM_CONTAINER)
@OslcResourceShape(title = "Tracked Resource Set Base Shape", describes = LDP_CONTAINER)
public class Base extends AbstractResource {
private List<URI> members;
private URI cutoffEvent;
private Page nextPage;
private final List<URI> members = new CopyOnWriteArrayList<>(); // thread-safe
private URI cutoffEvent;
private Page nextPage;

/**
* @return the members
*/
@OslcName(RDFS_TERM_MEMBER)
@OslcDescription("A member Resource of the Resource Set.")
@OslcPropertyDefinition(RDFS_MEMBER)
@OslcTitle("Member")
public List<URI> getMembers() {
if(members == null){
members = new ArrayList<>();
}
return members;
}
/**
* @return the members
*/
@OslcName(RDFS_TERM_MEMBER)
@OslcDescription("A member Resource of the Resource Set.")
@OslcPropertyDefinition(RDFS_MEMBER)
@OslcTitle("Member")
public List<URI> getMembers() {
Copy link
Copy Markdown
Contributor

@berezovskyi berezovskyi Dec 5, 2025

Choose a reason for hiding this comment

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

Can we update this signature to return a readonly list? To stop the pattern of getting a list and then modifying it by hand.

Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

getMembers exposes the internal representation stored in field members. The value may be modified after this call to getMembers.
getMembers exposes the internal representation stored in field members. The value may be modified after this call to getMembers.
getMembers exposes the internal representation stored in field members. The value may be modified after this call to getMembers.
getMembers exposes the internal representation stored in field members. The value may be modified after this call to getMembers.
getMembers exposes the internal representation stored in field members. The value may be modified after this call to getMembers.
getMembers exposes the internal representation stored in field members. The value may be modified after this call to getMembers.
getMembers exposes the internal representation stored in field members. The value may be modified after this call to getMembers.
getMembers exposes the internal representation stored in field members. The value may be modified after this call to getMembers.
getMembers exposes the internal representation stored in field members. The value may be modified after this call to getMembers.
getMembers exposes the internal representation stored in field members. The value may be modified after this call to getMembers.

Copilot uses AI. Check for mistakes.
return members;
}

/**
* @param members the members to set
*/
public void setMembers(List<URI> members) {
this.members = members;
}
/**
* @param members the members to set
*/
public void setMembers(List<URI> members) {
if (members == null) {
throw new IllegalArgumentException("Members list must not be null");
}
this.members.clear();
Copy link
Copy Markdown
Contributor

@berezovskyi berezovskyi Dec 5, 2025

Choose a reason for hiding this comment

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

This does not seem thread safe. As no lock is used, there could be a case where a reader would observe an empty list while this update is going on. Can we not just introduce an api to add an item? Otherwise it's best to mark this method synchronised.

this.members.addAll(members);
}

/**
* @return the cutoffIdentifier
*/
@OslcName(TRS_TERM_CUTOFFEVENT)
@OslcDescription("The most recent Change Log entry that is accounted for in this Base. When rdf:nil, the Base is an enumeration at the start of time.")
@OslcPropertyDefinition(TRS_CUTOFFEVENT)
@OslcTitle("Cutoff Event")
public URI getCutoffEvent() {
return cutoffEvent;
}
/**
* @param cutoffEvent the cutoffEvent to set
*/
public void setCutoffEvent(URI cutoffEvent) {
this.cutoffEvent = cutoffEvent;
}
/**
* @return the cutoffIdentifier
*/
@OslcName(TRS_TERM_CUTOFFEVENT)
@OslcDescription("The most recent Change Log entry that is accounted for in this Base. When rdf:nil, the Base is an enumeration at the start of time.")
@OslcPropertyDefinition(TRS_CUTOFFEVENT)
@OslcTitle("Cutoff Event")
public URI getCutoffEvent() {
return cutoffEvent;
}
/**
* @param cutoffEvent the cutoffEvent to set
*/
public void setCutoffEvent(URI cutoffEvent) {
this.cutoffEvent = cutoffEvent;
}

/**
* Return a Page object containing information about the next base page.
* The OslcHidden annotation works around a limitation in OSLC4J. If we do
* not hide the nextPage variable we get an incorrect ldp:nextPage reference
* in the Base.
*
* @return the nextPage
*/
@OslcHidden(value=true)
public Page getNextPage() {
return nextPage;
}
/**
* Return a Page object containing information about the next base page.
* The OslcHidden annotation works around a limitation in OSLC4J. If we do
* not hide the nextPage variable we get an incorrect ldp:nextPage reference
* in the Base.
*
* @return the nextPage
*/
@OslcHidden(value=true)
public Page getNextPage() {
return nextPage;
}

/**
* @param nextPage the nextPage to set
*/
public void setNextPage(Page nextPage) {
this.nextPage = nextPage;
}
/**
* @param nextPage the nextPage to set
*/
public void setNextPage(Page nextPage) {
this.nextPage = nextPage;
}
}
42 changes: 22 additions & 20 deletions core/oslc-trs/src/main/java/org/eclipse/lyo/core/trs/ChangeLog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.net.URI;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;

import org.eclipse.lyo.oslc4j.core.annotation.OslcDescription;
import org.eclipse.lyo.oslc4j.core.annotation.OslcName;
Expand All @@ -42,29 +43,29 @@
*
* {@code
* <http://cm1.example.com/trackedResourceSet>
* a trs:TrackedResourceSet ;
* trs:base <http://cm1.example.com/baseResources> ;
* trs:changeLog [
* a trs:ChangeLog ;
* trs:change <urn:urn-3:cm1.example.com:2010-10-27T17:39:33.000Z:103> ;
* trs:change <urn:urn-3:cm1.example.com:2010-10-27T17:39:32.000Z:102> ;
* trs:change <urn:urn-3:cm1.example.com:2010-10-27T17:39:31.000Z:101> .
* ] .
* a trs:TrackedResourceSet ;
* trs:base <http://cm1.example.com/baseResources> ;
* trs:changeLog [
* a trs:ChangeLog ;
* trs:change <urn:urn-3:cm1.example.com:2010-10-27T17:39:33.000Z:103> ;
* trs:change <urn:urn-3:cm1.example.com:2010-10-27T17:39:32.000Z:102> ;
* trs:change <urn:urn-3:cm1.example.com:2010-10-27T17:39:31.000Z:101> .
* ] .
*
* <urn:urn-3:cm1.example.com:2010-10-27T17:39:33.000Z:103>
* a trs:Creation ;
* trs:changed <http://cm1.example.com/bugs/23> ;
* trs:order "103"^^xsd:integer .
* a trs:Creation ;
* trs:changed <http://cm1.example.com/bugs/23> ;
* trs:order "103"^^xsd:integer .
*
* <urn:urn-3:cm1.example.com:2010-10-27T17:39:32.000Z:102>
* a trs:Modification ;
* trs:changed <http://cm1.example.com/bugs/22> ;
* trs:order "102"^^xsd:integer .
* a trs:Modification ;
* trs:changed <http://cm1.example.com/bugs/22> ;
* trs:order "102"^^xsd:integer .
*
* <urn:urn-3:cm1.example.com:2010-10-27T17:39:31.000Z:101>
* a trs:Deletion ;
* trs:changed <http://cm1.example.com/bugs/21> ;
* trs:order "101"^^xsd:integer .
* a trs:Deletion ;
* trs:changed <http://cm1.example.com/bugs/21> ;
* trs:order "101"^^xsd:integer .
* }
* </pre>
*
Expand Down Expand Up @@ -121,9 +122,9 @@
* Resource that is not a member of the Resource Set.
*/
@OslcNamespace(TRS_NAMESPACE)
@OslcResourceShape(title = "Change Log Shape", describes = TRS_TYPE_CHANGE_LOG)
@OslcResourceShape(title = "Change Log Shape", describes = TRS_TYPE_CHANGE_LOG)
public class ChangeLog extends AbstractChangeLog {
private List<ChangeEvent> change = new ArrayList<>();
private final List<ChangeEvent> change = new CopyOnWriteArrayList<>(); // thread-safe
private URI previous;

/**
Expand All @@ -144,7 +145,8 @@ public void setChange(List<ChangeEvent> change) {
if (change == null) {
throw new IllegalArgumentException("Change Event list must not be null");
}
this.change = change;
this.change.clear();
this.change.addAll(change);
}

/**
Expand Down
6 changes: 3 additions & 3 deletions trs/client/trs-client/src/main/java/org/eclipse/lyo/trs/client/util/ProviderUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public static List<ChangeEvent> optimizedChangesList(List<ChangeLog> changeLogs,

// sort the events, needed for the cut-off later
firstChangelogEvents.sort(Comparator.comparing(ChangeEvent::getOrder));
firstChangeLog.setChange(firstChangelogEvents);
firstChangeLog.setChange(new ArrayList<>(firstChangelogEvents));

// TODO Andrew@2018-02-28: just delete this line, getter after setter is some superstition
// firstChangelogEvents = firstChangeLog.getChange();
Expand All @@ -100,8 +100,8 @@ public static List<ChangeEvent> optimizedChangesList(List<ChangeLog> changeLogs,
}
}

firstChangelogEvents = firstChangelogEvents.subList(indexOfSync + 1,
firstChangelogEvents.size());
firstChangelogEvents = new ArrayList<>(firstChangelogEvents.subList(indexOfSync + 1,
Copy link
Copy Markdown
Contributor

@berezovskyi berezovskyi Dec 5, 2025

Choose a reason for hiding this comment

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

I don’t like this, it's really prone to misuse. I can see how a user may forget to do this and get occasional concurrency issues.

Copy link
Copy Markdown
Contributor

@berezovskyi berezovskyi Dec 5, 2025

Choose a reason for hiding this comment

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

I also think that copy on write list AND a new arraylist are quite wasteful when used together?

firstChangelogEvents.size()));
firstChangeLog.setChange(firstChangelogEvents);

List<ChangeEvent> changesToProcess = new ArrayList<>();
Expand Down
5 changes: 5 additions & 0 deletions trs/server/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.vintage</groupId>
<artifactId>junit-vintage-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
Expand Down
Loading