Skip to content

BEV-111 Use service param value for limiting rows fetched #66

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

Open
wants to merge 1 commit into
base: BEV-111-Set-limit-on-evets-query-db
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Copy link
Contributor

Choose a reason for hiding this comment

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

Am i reading this right, that you are setting maxAuditTrails to 10.000? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am. It was unlimited before, so it should perform the same (or better if there are really more than 10 000 rows).

Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,15 @@ public AuditTrailService(
* @param action Restrict the results to only be about this type of action
* @param fingerprint The fingerprint
* @param operationID Restrict the results to only this operationID
* @param maxAuditTrails The max number of audit trails to fetch from database
* @return an iterator to all AuditTrailEvents matching the criteria from the parameters
*/
public AuditEventIterator queryAuditTrailEventsByIterator(Date fromDate, Date toDate, String fileID,
String collectionID, String reportingComponent, String actor,
FileAction action,
String fingerprint, String operationID) {
String fingerprint, String operationID, int maxAuditTrails) {
return store.getAuditTrailsByIterator(fileID, collectionID, reportingComponent, null, null, actor, action,
fromDate, toDate, fingerprint, operationID);
fromDate, toDate, fingerprint, operationID, maxAuditTrails);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* #%L
* Bitrepository Audit Trail Service
* %%
* Copyright (C) 2010 - 2012 The State and University Library, The Royal Library and The State Archives, Denmark
* Copyright (C) 2010 - 2025 Royal Danish Library and The State Archives, Denmark
* %%
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as
Expand All @@ -28,9 +28,11 @@
import org.slf4j.LoggerFactory;

import java.sql.PreparedStatement;
import java.util.ArrayList;
import java.util.Arrays;
import java.sql.SQLException;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.bitrepository.audittrails.store.AuditDatabaseConstants.ACTOR_KEY;
import static org.bitrepository.audittrails.store.AuditDatabaseConstants.ACTOR_NAME;
Expand Down Expand Up @@ -64,15 +66,11 @@
* As such any change in extraction model should be reflected in the AuditEventIterator.
* For further details @see {@link org.bitrepository.audittrails.store.AuditEventIterator}
* <p>
* <p>
* Order of extraction:
* FileId, ContributorId, SequenceNumber, SeqNumber, ActorName, Operation, OperationDate,
* AuditTrail, Information, OperationID, Certificate fingerprint
*/
public class AuditDatabaseExtractor {
/**
* The log.
*/
private final Logger log = LoggerFactory.getLogger(getClass());

/**
Expand Down Expand Up @@ -126,12 +124,10 @@ public class AuditDatabaseExtractor {
private final DBConnector dbConnector;

/**
* Constructor.
*
* @param model The model for the restriction for the extraction from the database.
* @param dbConnector The connector to the database, where the audit trails are to be extracted.
*/
public AuditDatabaseExtractor(ExtractModel model, DBConnector dbConnector) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ExtractModel isn’t visible outside the package, so no one could benefit from this constructor being public.

AuditDatabaseExtractor(ExtractModel model, DBConnector dbConnector) {
ArgumentValidator.checkNotNull(model, "ExtractModel model");
ArgumentValidator.checkNotNull(dbConnector, "DBConnector dbConnector");

Expand All @@ -145,16 +141,19 @@ public AuditDatabaseExtractor(ExtractModel model, DBConnector dbConnector) {
* @return {@link AuditEventIterator} Iterator for extracting the AuditTrails
*/
public AuditEventIterator extractAuditEventsByIterator() {
String sql = createSelectString() + " FROM " + AUDIT_TRAIL_TABLE + joinWithFileTable() + joinWithActorTable()
+ joinWithContributorTable() + createRestriction()
+ " ORDER BY " + AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_OPERATION_DATE + " FETCH FIRST 1000 ROWS ONLY";
String sql = createSelectString() +
" FROM " + AUDIT_TRAIL_TABLE + joinWithFileTable() + joinWithActorTable() + joinWithContributorTable() +
createRestriction() +
" ORDER BY " + AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_OPERATION_DATE +
createRowLimit();
try {
List<Object> arguments = extractArgumentsFromModel();
log.debug("Creating prepared statement with sql '{}' and arguments '{}' for AuditEventIterator",
sql, Arrays.asList(extractArgumentsFromModel()));
sql, arguments);
PreparedStatement ps = DatabaseUtils.createPreparedStatement(dbConnector.getConnection(),
sql, extractArgumentsFromModel());
sql, arguments.toArray());
return new AuditEventIterator(ps);
} catch (Exception e) {
} catch (SQLException | RuntimeException e) {
throw new IllegalStateException("Failed to retrieve the audit trails from the database", e);
}
}
Expand All @@ -166,21 +165,17 @@ public AuditEventIterator extractAuditEventsByIterator() {
* @return Creates the SELECT string for the retrieval of the audit events.
*/
private String createSelectString() {
StringBuilder res = new StringBuilder();

res.append("SELECT ");
res.append(FILE_TABLE + "." + FILE_FILE_ID + ", ");
res.append(CONTRIBUTOR_TABLE + "." + CONTRIBUTOR_ID + ", ");
res.append(AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_SEQUENCE_NUMBER + ", ");
res.append(ACTOR_TABLE + "." + ACTOR_NAME + ", ");
res.append(AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_OPERATION + ", ");
res.append(AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_OPERATION_DATE + ", ");
res.append(AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_AUDIT + ", ");
res.append(AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_INFORMATION + ", ");
res.append(AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_OPERATION_ID + ", ");
res.append(AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_FINGERPRINT + " ");

return res.toString();
return "SELECT " +
FILE_TABLE + "." + FILE_FILE_ID + ", " +
CONTRIBUTOR_TABLE + "." + CONTRIBUTOR_ID + ", " +
AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_SEQUENCE_NUMBER + ", " +
ACTOR_TABLE + "." + ACTOR_NAME + ", " +
AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_OPERATION + ", " +
AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_OPERATION_DATE + ", " +
AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_AUDIT + ", " +
AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_INFORMATION + ", " +
AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_OPERATION_ID + ", " +
AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_FINGERPRINT + " ";
}

/**
Expand All @@ -189,8 +184,8 @@ private String createSelectString() {
* @return The sql for joining the tables.
*/
private String joinWithFileTable() {
return " JOIN " + FILE_TABLE + " ON " + AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_FILE_KEY + " = " + FILE_TABLE + "."
+ FILE_KEY + " ";
return " JOIN " + FILE_TABLE +
" ON " + AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_FILE_KEY + " = " + FILE_TABLE + "." + FILE_KEY + " ";
}

/**
Expand All @@ -199,8 +194,8 @@ private String joinWithFileTable() {
* @return The sql for joining the tables.
*/
private String joinWithActorTable() {
return " JOIN " + ACTOR_TABLE + " ON " + AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_ACTOR_KEY + " = " + ACTOR_TABLE
+ "." + ACTOR_KEY + " ";
return " JOIN " + ACTOR_TABLE +
" ON " + AUDIT_TRAIL_TABLE + "." + AUDIT_TRAIL_ACTOR_KEY + " = " + ACTOR_TABLE + "." + ACTOR_KEY + " ";
}

/**
Expand Down Expand Up @@ -295,56 +290,26 @@ private void nextArgument(StringBuilder res) {
}
}

/**
* @return The list of elements in the model which are not null.
*/
private Object[] extractArgumentsFromModel() {
List<Object> res = new ArrayList<>();

if (model.getFileID() != null) {
res.add(model.getFileID());
}

if (model.getCollectionID() != null) {
res.add(model.getCollectionID());
}

if (model.getContributorID() != null) {
res.add(model.getContributorID());
}

if (model.getMinSeqNumber() != null) {
res.add(model.getMinSeqNumber());
}

if (model.getMaxSeqNumber() != null) {
res.add(model.getMaxSeqNumber());
}

if (model.getActorName() != null) {
res.add(model.getActorName());
}

if (model.getOperation() != null) {
res.add(model.getOperation().toString());
}

if (model.getStartDate() != null) {
res.add(model.getStartDate().getTime());
}

if (model.getEndDate() != null) {
res.add(model.getEndDate().getTime());
}

if (model.getFingerprint() != null) {
res.add(model.getFingerprint());
}

if (model.getOperationID() != null) {
res.add(model.getOperationID());
private String createRowLimit() {
if (model.getMaxAuditTrails() == null) {
return "";
}
return " FETCH FIRST ? ROWS ONLY";
}

return res.toArray();
/**
* @return The list of elements in the model which are not null,
* converted to types applicable for DatabaseUtils where appropriate.
*/
private List<Object> extractArgumentsFromModel() {
return Stream.of(model.getFileID(), model.getCollectionID(), model.getContributorID(),
model.getMinSeqNumber(), model.getMaxSeqNumber(), model.getActorName(),
model.getOperation() == null ? null : model.getOperation().toString(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not completely sure here. The method is much terser now, but these lines aren’t beautiful, so is it worth it? I generally clearly prefer readable code over short code.

model.getStartDate() == null ? null : model.getStartDate().getTime(),
model.getEndDate() == null ? null : model.getEndDate().getTime(),
model.getFingerprint(), model.getOperationID(), model.getMaxAuditTrails())
.filter(Objects::nonNull)
.collect(Collectors.toList());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,30 @@ public AuditTrailServiceDAO(DatabaseManager databaseManager) {

@Override
public AuditEventIterator getAuditTrailsByIterator(String fileID, String collectionID, String contributorID,
Long minSeqNumber, Long maxSeqNumber, String actorName, FileAction operation,
Date startDate,
Date endDate, String fingerprint, String operationID) {
Long minSeqNumber, Long maxSeqNumber, String actorName,
FileAction operation,
Date startDate, Date endDate, String fingerprint,
String operationID) {
return getAuditEventIteratorImplementation(fileID, collectionID, contributorID, minSeqNumber, maxSeqNumber,
actorName, operation, startDate, endDate, fingerprint, operationID, null);
}

@Override
public AuditEventIterator getAuditTrailsByIterator(String fileID, String collectionID, String contributorID,
Long minSeqNumber, Long maxSeqNumber, String actorName,
FileAction operation,
Date startDate, Date endDate, String fingerprint,
String operationID, int maxAuditTrails) {
return getAuditEventIteratorImplementation(fileID, collectionID, contributorID, minSeqNumber, maxSeqNumber,
actorName, operation, startDate, endDate, fingerprint, operationID, maxAuditTrails);
}

private AuditEventIterator getAuditEventIteratorImplementation(String fileID, String collectionID,
String contributorID, Long minSeqNumber,
Long maxSeqNumber, String actorName,
FileAction operation, Date startDate, Date endDate,
String fingerprint, String operationID,
Integer maxAuditTrails) {
ExtractModel model = new ExtractModel();
model.setFileID(fileID);
model.setCollectionID(collectionID);
Expand All @@ -65,6 +86,7 @@ public AuditEventIterator getAuditTrailsByIterator(String fileID, String collect
model.setEndDate(endDate);
model.setFingerprint(fingerprint);
model.setOperationID(operationID);
model.setMaxAuditTrails(maxAuditTrails);

AuditDatabaseExtractor extractor = new AuditDatabaseExtractor(model, dbConnector);
return extractor.extractAuditEventsByIterator();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
* Interface for the storage of audit trail information for the AuditTrailService.
*/
public interface AuditTrailStore {

/**
* Obtain AuditEventIterator for extracting audit trails from the store.
* When done with the iterator, the user should ensure that it is closed.
Expand All @@ -56,6 +57,29 @@ AuditEventIterator getAuditTrailsByIterator(String fileID, String collectionID,
Date startDate,
Date endDate, String fingerprint, String operationID);

/**
* Obtain AuditEventIterator for extracting audit trails from the store.
* When done with the iterator, the user should ensure that it is closed.
*
* @param fileID [OPTIONAL] The id of the file for restricting the extraction.
* @param collectionID [OPTIONAL] The id of the collection from which to retrieve audit trails.
* @param contributorID [OPTIONAL] The id of the contributor for restricting the extraction.
* @param minSeqNumber [OPTIONAL] The minimum sequence number for restricting the extraction.
* @param maxSeqNumber [OPTIONAL] The maximum sequence number for restricting the extraction.
* @param actorName [OPTIONAL] The name of the actor for restricting the extraction.
* @param operation [OPTIONAL] The FileAction operation for restricting the extraction.
* @param startDate [OPTIONAL] The earliest date for the audits for restricting the extraction.
* @param endDate [OPTIONAL] The latest date for the audits for restricting the extraction.
* @param fingerprint [OPTIONAL] The fingerprint of the certificate for the audits
* @param operationID [OPTIONAL] The ID of the operation (conversationID) for the audits
* @param maxAuditTrails The max number of audit trails to fetch from database.
* @return The requested audit trails from the store.
*/
AuditEventIterator getAuditTrailsByIterator(String fileID, String collectionID, String contributorID,
Long minSeqNumber, Long maxSeqNumber, String actorName, FileAction operation,
Date startDate,
Date endDate, String fingerprint, String operationID, int maxAuditTrails);

/**
* ingest audit trails into the store.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ class ExtractModel {
*/
private String operationID;

public ExtractModel() {
}
private Integer maxAuditTrails;

/**
* @return The fileID;
Expand Down Expand Up @@ -233,4 +232,19 @@ public String getOperationID() {
public void setOperationID(String operationID) {
this.operationID = operationID;
}

/**
* @return The max number of audit trails to fetch from database or null for unlimited
*/
public Integer getMaxAuditTrails() {
return maxAuditTrails;
}

/**
* @param maxAuditTrails The max number of audit trails to fetch from database or null for unlimited
*/
public void setMaxAuditTrails(Integer maxAuditTrails) {
this.maxAuditTrails = maxAuditTrails;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
import org.bitrepository.common.utils.TimeUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.ws.rs.Consumes;
import javax.ws.rs.DefaultValue;
import javax.ws.rs.FormParam;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
Expand Down Expand Up @@ -77,14 +79,14 @@ public StreamingOutput queryAuditTrailEvents(@FormParam("fromDate") String fromD
@FormParam("collectionID") String collectionID,
@FormParam("fingerprint") String fingerprint,
@FormParam("operationID") String operationID,
@FormParam("maxAuditTrails") Integer maxResults) {
@DefaultValue("1000") @FormParam("maxAuditTrails") Integer maxResults) {
Date from = calendarUtils.makeStartDateObject(fromDate);
Date to = calendarUtils.makeEndDateObject(toDate);

final int maxAudits = maxResults;
final AuditEventIterator it = service.queryAuditTrailEventsByIterator(from, to, contentOrNull(fileID),
collectionID, contentOrNull(reportingComponent), contentOrNull(actor), filterAction(action),
contentOrNull(fingerprint), contentOrNull(operationID));
contentOrNull(fingerprint), contentOrNull(operationID), maxAudits);
if (it != null) {
return output -> {
JsonFactory jf = new JsonFactory();
Expand All @@ -93,14 +95,14 @@ collectionID, contentOrNull(reportingComponent), contentOrNull(actor), filterAct
AuditTrailEvent event;
jg.writeStartArray();
int numAudits = 0;
while ((event = it.getNextAuditTrailEvent()) != null && numAudits < maxAudits) {
while ((event = it.getNextAuditTrailEvent()) != null) {
writeAuditResult(event, jg);
numAudits++;
}
jg.writeEndArray();
jg.flush();
jg.close();
} catch (Exception e) {
} catch (RuntimeException e) {
log.error("Caught exception trying to stream audit trails", e);
throw new WebApplicationException(e);
} finally {
Expand All @@ -114,8 +116,10 @@ collectionID, contentOrNull(reportingComponent), contentOrNull(actor), filterAct
};
} else {
throw new WebApplicationException(
Response.status(Response.Status.NO_CONTENT).entity("Failed to get audit trails from database")
.type(MediaType.TEXT_PLAIN).build());
Response.status(Response.Status.NO_CONTENT)
.entity("Failed to get audit trails from database: it is null")
.type(MediaType.TEXT_PLAIN)
.build());
}
}

Expand Down
Loading