Skip to content

Cache content not found #707

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
merged 3 commits into from
Jul 15, 2025
Merged

Cache content not found #707

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
b6a9306
Use Optional in getContentDOById to cache not found
jsharkey13 Jul 21, 2023
86cc41b
Remove unused glossary endpoint and cached method
jsharkey13 Jul 4, 2025
55e4bc2
Always cache ResultsWrappers and separate DO/DTOs
jsharkey13 Jul 4, 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
37 changes: 0 additions & 37 deletions src/main/java/uk/ac/cam/cl/dtg/segue/api/GlossaryFacade.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,11 @@
import uk.ac.cam.cl.dtg.isaac.dto.SegueErrorResponse;
import uk.ac.cam.cl.dtg.isaac.dto.content.ContentDTO;
import uk.ac.cam.cl.dtg.segue.dao.ILogManager;
import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException;
import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager;
import uk.ac.cam.cl.dtg.util.AbstractConfigLoader;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.QueryParam;
import jakarta.ws.rs.core.Context;
Expand Down Expand Up @@ -151,39 +149,4 @@ public final Response getTerms(@Context final Request request, @QueryParam("star
return new SegueErrorResponse(Status.INTERNAL_SERVER_ERROR, "Error loading glossary terms!").toResponse();
}
}

/**
* Gets the current version of the segue application.
*
* @param term_id - The ID of the term to retrieve.
*
* @return segue version as a string wrapped in a response.
*/
@GET
@Path("terms/{term_id}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that this endpoint is gone, I suggest also deleting https://github.com/isaacphysics/isaac-react-app/blob/7b33e847ea1a968d83e1b2dd882091af641a7694/src/app/services/api.ts#L151

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, that was on my radar for once this is merged.

@Produces(MediaType.APPLICATION_JSON)
@Operation(summary = "Get the term with the given id.")
public final Response getTermById(@PathParam("term_id") final String term_id) {

if (null == term_id) {
return new SegueErrorResponse(Status.BAD_REQUEST, "Please specify a term_id.").toResponse();
}

ResultsWrapper<ContentDTO> c;
try {
c = this.contentManager.getUnsafeCachedDTOsByIdPrefix(term_id, 0, 10000);
if (null == c) {
SegueErrorResponse error = new SegueErrorResponse(Status.NOT_FOUND, "No glossary term found with id: " + term_id);
log.debug(error.getErrorMessage());
return error.toResponse();
}
} catch (ContentManagerException e) {
return new SegueErrorResponse(Status.INTERNAL_SERVER_ERROR,
"Content acquisition error.", e).toResponse();
}
// Calculate the ETag on last modified date of tags list
// NOTE: Assumes that the latest version of the content is being used.
EntityTag etag = new EntityTag(this.contentManager.getCurrentContentSHA().hashCode() + "");
return Response.ok(c).tag(etag).build();
}
}
132 changes: 55 additions & 77 deletions src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

Expand All @@ -88,7 +89,8 @@
private final boolean showOnlyPublishedContent;
private final boolean hideRegressionTestContent;

private final Cache<Object, Object> cache;
private final Cache<String, ResultsWrapper<Content>> contentDOcache;
private final Cache<String, ResultsWrapper<ContentDTO>> contentDTOcache;
private final Cache<String, GetResponse> contentShaCache;

private final String contentIndex;
Expand Down Expand Up @@ -126,8 +128,10 @@
log.info("API Configured to hide content tagged with 'regression_test'.");
}

this.cache = CacheBuilder.newBuilder().recordStats().softValues().expireAfterAccess(1, TimeUnit.DAYS).build();
CACHE_METRICS_COLLECTOR.addCache("git_content_manager_cache", cache);
this.contentDOcache = CacheBuilder.newBuilder().recordStats().softValues().expireAfterAccess(1, TimeUnit.DAYS).build();
this.contentDTOcache = CacheBuilder.newBuilder().recordStats().softValues().expireAfterAccess(1, TimeUnit.DAYS).build();
CACHE_METRICS_COLLECTOR.addCache("git_content_manager_do_cache", contentDOcache);
CACHE_METRICS_COLLECTOR.addCache("git_content_manager_dto_cache", contentDTOcache);

this.contentShaCache = CacheBuilder.newBuilder().softValues().expireAfterWrite(5, TimeUnit.SECONDS).build();

Expand All @@ -152,7 +156,8 @@
this.globalProperties = null;
this.showOnlyPublishedContent = false;
this.hideRegressionTestContent = false;
this.cache = CacheBuilder.newBuilder().softValues().expireAfterAccess(1, TimeUnit.DAYS).build();
this.contentDOcache = CacheBuilder.newBuilder().recordStats().softValues().expireAfterAccess(1, TimeUnit.DAYS).build();
this.contentDTOcache = CacheBuilder.newBuilder().recordStats().softValues().expireAfterAccess(1, TimeUnit.DAYS).build();
this.contentShaCache = CacheBuilder.newBuilder().softValues().expireAfterWrite(1, TimeUnit.MINUTES).build();
this.contentIndex = null;
}
Expand Down Expand Up @@ -235,69 +240,40 @@
}

String k = "getContentDOById~" + getCurrentContentSHA() + "~" + id;
if (!cache.asMap().containsKey(k)) {

List<Content> searchResults = mapper.mapFromStringListToContentList(this.searchProvider.termSearch(
contentIndex,
CONTENT_TYPE, id,
Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, 0, 1,
this.getBaseFilters()).getResults()
);
try {
ResultsWrapper<Content> result = contentDOcache.get(k, () -> {

ResultsWrapper<String> rawResults = searchProvider.termSearch(
contentIndex,
CONTENT_TYPE, id,
Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX, 0, 1,
getBaseFilters());
List<Content> searchResults = mapper.mapFromStringListToContentList(rawResults.getResults());

if (null == searchResults || searchResults.isEmpty()) {
return new ResultsWrapper<>(searchResults, rawResults.getTotalResults());
});

if (null == result.getResults() || result.getResults().isEmpty()) {
if (!failQuietly) {
log.error(String.format(
"Failed to locate content with ID '%s' in the cache for content SHA (%s)",
id, getCurrentContentSHA()
));
log.error("Failed to locate content with ID '{}' in the cache for content SHA ({})", id, getCurrentContentSHA());
}
return null;
}

cache.put(k, searchResults.get(0));
}

return (Content) cache.getIfPresent(k);

}

/**
* Retrieve all DTO content matching an ID prefix.
*
* This may return cached objects, and will temporarily cache the objects
* to avoid re-querying the data store and the deserialization costs.
* Do not modify the returned DTO objects.
*
* @param idPrefix the content object ID prefix.
* @param startIndex the integer start index for pagination.
* @param limit the limit for pagination.
* @return a ResultsWrapper of the matching content.
* @throws ContentManagerException on failure to return the objects.
*/
public ResultsWrapper<ContentDTO> getUnsafeCachedDTOsByIdPrefix(final String idPrefix, final int startIndex,
final int limit) throws ContentManagerException {

String k = "getByIdPrefix~" + getCurrentContentSHA() + "~" + idPrefix + "~" + startIndex + "~" + limit;
if (!cache.asMap().containsKey(k)) {

ResultsWrapper<String> searchHits = this.searchProvider.findByPrefix(contentIndex, CONTENT_TYPE,
Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX,
idPrefix, startIndex, limit, this.getBaseFilters());
return result.getResults().get(0);

List<Content> searchResults = mapper.mapFromStringListToContentList(searchHits.getResults());

cache.put(k, new ResultsWrapper<>(mapper.getDTOByDOList(searchResults), searchHits.getTotalResults()));
} catch (final ExecutionException e) {
throw new ContentManagerException(e.getCause().getMessage());
}

return (ResultsWrapper<ContentDTO>) cache.getIfPresent(k);
}

/**
* Get a list of DTO objects by their IDs.
*
* This may return cached objects, and will temporarily cache the objects
* to avoid re-querying the data store and the deserialization costs.
* Do not modify the returned DTO objects.
* This will always return cached objects, and temporarily caches the objects to avoid re-querying
* the data store and the deserialization costs.
* Do not modify the returned DTO objects!
*
* @param ids the list of content object IDs.
* @param startIndex the integer start index for pagination.
Expand All @@ -309,35 +285,37 @@
final int startIndex, final int limit)
throws ContentManagerException {

String k = "getContentMatchingIds~" + getCurrentContentSHA()
+ "~" + ids.toString() + "~" + startIndex + "~" + limit;
if (!cache.asMap().containsKey(k)) {
String k = "getContentMatchingIds~" + getCurrentContentSHA() + "~" + ids.toString() + "~" + startIndex + "~" + limit;

Map<String, AbstractFilterInstruction> finalFilter = Maps.newHashMap();
finalFilter.putAll(new ImmutableMap.Builder<String, AbstractFilterInstruction>()
.put(Constants.ID_FIELDNAME + "." + Constants.UNPROCESSED_SEARCH_FIELD_SUFFIX,
new TermsFilterInstruction(ids))
.build());
try {
return contentDTOcache.get(k, () -> {

if (getBaseFilters() != null) {
finalFilter.putAll(getBaseFilters());
}
Map<String, AbstractFilterInstruction> finalFilter = Maps.newHashMap();
finalFilter.putAll(new ImmutableMap.Builder<String, AbstractFilterInstruction>()
.put(ID_FIELDNAME + "." + UNPROCESSED_SEARCH_FIELD_SUFFIX,
new TermsFilterInstruction(ids))
.build());

ResultsWrapper<String> searchHits = this.searchProvider.termSearch(
contentIndex,
CONTENT_TYPE,
null,
null,
startIndex,
limit,
finalFilter
);
if (getBaseFilters() != null) {
finalFilter.putAll(getBaseFilters());
}

List<Content> searchResults = mapper.mapFromStringListToContentList(searchHits.getResults());
cache.put(k, new ResultsWrapper<>(mapper.getDTOByDOList(searchResults), searchHits.getTotalResults()));
ResultsWrapper<String> searchHits = this.searchProvider.termSearch(
contentIndex,
CONTENT_TYPE,
null,
null,
startIndex,
limit,
finalFilter
);

List<Content> searchResults = mapper.mapFromStringListToContentList(searchHits.getResults());
return new ResultsWrapper<>(mapper.getDTOByDOList(searchResults), searchHits.getTotalResults());
});
} catch (final ExecutionException e) {
throw new ContentManagerException(e.getCause().getMessage());
}

return (ResultsWrapper<ContentDTO>) cache.getIfPresent(k);
}

/** Search the content for specified types that match a given user provided search string from a given index.
Expand Down
Loading