Skip to content

fix: Fix race conditions in repository refresh and Space field visibility #375

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
tkuhn merged 1 commit into from
Mar 5, 2026
Merged

fix: Fix race conditions in repository refresh and Space field visibility #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/main/java/com/knowledgepixels/nanodash/domain/Space.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ private static String getCoreInfoString(ApiResponseEntry resp) {
return id + " " + rootNanopubId;
}

private boolean dataInitialized = false;
private boolean dataNeedsUpdate = true;
private volatile boolean dataInitialized = false;
private volatile boolean dataNeedsUpdate = true;

Space(ApiResponseEntry resp) {
super(resp.get("space"));
Expand Down
28 changes: 17 additions & 11 deletions src/main/java/com/knowledgepixels/nanodash/repository/MaintainedResourceRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,29 +48,33 @@ private MaintainedResourceRepository() {
* @param resp The API response containing maintained resource data.
*/
public synchronized void refresh(ApiResponse resp) {
resourceList = new ArrayList<>();
resourcesById = new HashMap<>();
resourcesBySpace = new HashMap<>();
resourcesByNamespace = new HashMap<>();
List<MaintainedResource> newResourceList = new ArrayList<>();
Map<String, MaintainedResource> newResourcesById = new HashMap<>();
Map<Space, List<MaintainedResource>> newResourcesBySpace = new HashMap<>();
Map<String, MaintainedResource> newResourcesByNamespace = new HashMap<>();
for (ApiResponseEntry entry : resp.getData()) {
Space space = SpaceRepository.get().findById(entry.get("space"));
if (space == null) {
continue;
}
MaintainedResource resource = MaintainedResourceFactory.getOrCreate(entry, space);
if (resourcesById.containsKey(resource.getId())) {
if (newResourcesById.containsKey(resource.getId())) {
continue;
}
resourceList.add(resource);
resourcesById.put(resource.getId(), resource);
resourcesBySpace.computeIfAbsent(space, k -> new ArrayList<>()).add(resource);
newResourceList.add(resource);
newResourcesById.put(resource.getId(), resource);
newResourcesBySpace.computeIfAbsent(space, k -> new ArrayList<>()).add(resource);
if (resource.getNamespace() != null) {
// TODO Handle conflicts when two resources claim the same namespace:
resourcesByNamespace.put(resource.getNamespace(), resource);
newResourcesByNamespace.put(resource.getNamespace(), resource);
}
}
MaintainedResourceFactory.removeStale(resourcesById.keySet());
MaintainedResourceFactory.removeStale(newResourcesById.keySet());
resourcesById = newResourcesById;
resourcesBySpace = newResourcesBySpace;
resourcesByNamespace = newResourcesByNamespace;
loaded = true;
resourceList = newResourceList; // volatile write last — establishes happens-before for all above
}

/**
Expand Down Expand Up @@ -121,7 +125,9 @@ public void ensureLoaded() {
} catch (InterruptedException ex) {
logger.error("Interrupted", ex);
}
refresh(ApiCache.retrieveResponseSync(new QueryRef(QueryApiAccess.GET_MAINTAINED_RESOURCES), true));
if (resourceList == null) { // double-check after potential wait
refresh(ApiCache.retrieveResponseSync(new QueryRef(QueryApiAccess.GET_MAINTAINED_RESOURCES), true));
}
}
}

Expand Down
40 changes: 25 additions & 15 deletions src/main/java/com/knowledgepixels/nanodash/repository/SpaceRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public static SpaceRepository get() {
private Map<Space, Set<Space>> subspaceMap;
private Map<Space, Set<Space>> superspaceMap;
private boolean loaded = false;
private Long runRootUpdateAfter = null;
private volatile Long runRootUpdateAfter = null;

private final Object loadLock = new Object();

Expand All @@ -51,27 +51,35 @@ private SpaceRepository() {
*/
public synchronized void refresh(ApiResponse resp) {
logger.info("Refreshing spaces from API response with {} entries", resp.getData().size());
spaceList = new ArrayList<>();
spaceListByType = new HashMap<>();
spacesById = new HashMap<>();
subspaceMap = new HashMap<>();
superspaceMap = new HashMap<>();
List<Space> newSpaceList = new ArrayList<>();
Map<String, List<Space>> newSpaceListByType = new HashMap<>();
Map<String, Space> newSpacesById = new HashMap<>();
Map<Space, Set<Space>> newSubspaceMap = new HashMap<>();
Map<Space, Set<Space>> newSuperspaceMap = new HashMap<>();
for (ApiResponseEntry entry : resp.getData()) {
Space space;
space = SpaceFactory.getOrCreate(entry);
spaceList.add(space);
spaceListByType.computeIfAbsent(space.getType(), k -> new ArrayList<>()).add(space);
spacesById.put(space.getId(), space);
newSpaceList.add(space);
newSpaceListByType.computeIfAbsent(space.getType(), k -> new ArrayList<>()).add(space);
newSpacesById.put(space.getId(), space);
}
SpaceFactory.removeStale(spacesById.keySet());
for (Space space : spaceList) {
Space superSpace = this.getIdSuperspace(space);
SpaceFactory.removeStale(newSpacesById.keySet());
for (Space space : newSpaceList) {
String id = space.getId();
if (!id.matches("https?://[^/]+/.*/[^/]*/?")) continue;
String superId = id.replaceFirst("(https?://[^/]+/.*)/[^/]*/?", "$1");
Space superSpace = newSpacesById.get(superId);
if (superSpace == null) continue;
subspaceMap.computeIfAbsent(superSpace, k -> new HashSet<>()).add(space);
superspaceMap.computeIfAbsent(space, k -> new HashSet<>()).add(superSpace);
newSubspaceMap.computeIfAbsent(superSpace, k -> new HashSet<>()).add(space);
newSuperspaceMap.computeIfAbsent(space, k -> new HashSet<>()).add(superSpace);
space.setDataNeedsUpdate();
}
spacesById = newSpacesById;
spaceListByType = newSpaceListByType;
subspaceMap = newSubspaceMap;
superspaceMap = newSuperspaceMap;
loaded = true;
spaceList = newSpaceList; // volatile write last — establishes happens-before for all above
}

/**
Expand Down Expand Up @@ -101,7 +109,9 @@ public void ensureLoaded() {
} catch (InterruptedException ex) {
logger.error("Interrupted", ex);
}
refresh(ApiCache.retrieveResponseSync(new QueryRef(QueryApiAccess.GET_SPACES), true));
if (spaceList == null) { // double-check after potential wait
refresh(ApiCache.retrieveResponseSync(new QueryRef(QueryApiAccess.GET_SPACES), true));
}
}
}

Expand Down