code review

This commit is contained in:
frabacche
2023-09-04 13:30:11 +02:00
parent 184b14e66e
commit bbf7bb1ae0
15 changed files with 178 additions and 92 deletions

View File

@@ -29,9 +29,28 @@ import org.dspace.core.ReloadableEntity;
@Table(name = "ldn_message")
public class LDNMessageEntity implements ReloadableEntity<String> {
/**
* LDN messages interact with a fictitious queue. Scheduled tasks manage the queue.
*/
/**
* Message queued, it has to be elaborated.
*/
public static final Integer QUEUE_STATUS_QUEUED = 1;
/**
* Message has been taken from the queue and it's elaboration is in progress.
*/
public static final Integer QUEUE_STATUS_PROCESSING = 2;
/**
* Message has been correctly elaborated.
*/
public static final Integer QUEUE_STATUS_PROCESSED = 3;
/**
* Message has not been correctly elaborated - despite more than "ldn.processor.max.attempts" retryies
*/
public static final Integer QUEUE_STATUS_FAILED = 4;
@Id
@@ -77,6 +96,12 @@ public class LDNMessageEntity implements ReloadableEntity<String> {
@JoinColumn(name = "context", referencedColumnName = "uuid")
private DSpaceObject context;
@Column(name = "activity_stream_type")
private String activityStreamType;
@Column(name = "coar_notify_type")
private String coarNotifyType;
protected LDNMessageEntity() {
}
@@ -118,6 +143,22 @@ public class LDNMessageEntity implements ReloadableEntity<String> {
this.type = type;
}
public String getActivityStreamType() {
return activityStreamType;
}
public void setActivityStreamType(String activityStreamType) {
this.activityStreamType = activityStreamType;
}
public String getCoarNotifyType() {
return coarNotifyType;
}
public void setCoarNotifyType(String coarNotifyType) {
this.coarNotifyType = coarNotifyType;
}
public NotifyServiceEntity getOrigin() {
return origin;
}

View File

@@ -36,6 +36,7 @@ public class LDNQueueExtractor {
} else {
log.error("Errors happened during the extract operations. Check the log above!");
}
context.complete();
log.info("END LDNQueueExtractor.extractMessageFromQueue()");
return processed_messages;
}

View File

@@ -38,6 +38,7 @@ public class LDNQueueTimeoutChecker {
log.error("Errors happened during the check operation. Check the log above!");
}
log.info("END LDNQueueTimeoutChecker.checkQueueMessageTimeout()");
context.complete();
return fixed_messages;
}
}

View File

@@ -30,23 +30,17 @@ public class LDNRouter {
* @return LDNProcessor processor to process notification, can be null
*/
public LDNProcessor route(LDNMessageEntity ldnMessage) {
if (ldnMessage == null) {
log.warn("an null LDNMessage is received for routing!");
return null;
}
if (StringUtils.isEmpty(ldnMessage.getType())) {
log.warn("LDNMessage " + ldnMessage + " has no type!");
return null;
}
if (ldnMessage == null) {
log.warn("an null LDNMessage " + ldnMessage + "is received for routing!");
return null;
}
String ldnMessageType = ldnMessage.getType();
ldnMessageType = ldnMessageType.replace("[", "");
ldnMessageType = ldnMessageType.replace("]", "");
ldnMessageType = ldnMessageType.replace(" ", "");
String[] ldnMsgTypeArray = ldnMessageType.split(",");
Set<String> ldnMessageTypeSet = new HashSet<String>();
for (int i = 0; i < ldnMsgTypeArray.length; i++) {
ldnMessageTypeSet.add(ldnMsgTypeArray[i]);
}
ldnMessageTypeSet.add(ldnMessage.getActivityStreamType());
ldnMessageTypeSet.add(ldnMessage.getCoarNotifyType());
LDNProcessor processor = processors.get(ldnMessageTypeSet);
return processor;
}

View File

@@ -73,6 +73,9 @@ public class NotifyServiceEntity implements ReloadableEntity<Integer> {
this.description = description;
}
/**
* @return URL of an informative website
*/
public String getUrl() {
return url;
}
@@ -81,6 +84,9 @@ public class NotifyServiceEntity implements ReloadableEntity<Integer> {
this.url = url;
}
/**
* @return URL of the LDN InBox
*/
public String getLdnUrl() {
return ldnUrl;
}
@@ -89,6 +95,9 @@ public class NotifyServiceEntity implements ReloadableEntity<Integer> {
this.ldnUrl = ldnUrl;
}
/**
* @return The list of the inbound patterns configuration supported by the service
*/
public List<NotifyServiceInboundPattern> getInboundPatterns() {
return inboundPatterns;
}

View File

@@ -14,7 +14,7 @@ import org.dspace.services.factory.DSpaceServicesFactory;
* Abstract factory to get services for the NotifyService package,
* use NotifyServiceFactory.getInstance() to retrieve an implementation
*
* @author Mohamed Eskander (mohamed.eskander at 4science.com)
* @author Francesco Bacchelli (francesco.bacchelli at 4science.com)
*/
public abstract class LDNMessageServiceFactory {

View File

@@ -14,7 +14,7 @@ import org.springframework.beans.factory.annotation.Autowired;
* Factory implementation to get services for the notifyservices package, use
* NotifyServiceFactory.getInstance() to retrieve an implementation
*
* @author Mohamed Eskander (mohamed.eskander at 4science.com)
* @author Francesco Bacchelli (francesco.bacchelli at 4science.com)
*/
public class LDNMessageServiceFactoryImpl extends LDNMessageServiceFactory {

View File

@@ -14,7 +14,7 @@ import org.dspace.services.factory.DSpaceServicesFactory;
* Abstract factory to get services for the NotifyService package, use
* NotifyServiceFactory.getInstance() to retrieve an implementation
*
* @author Mohamed Eskander (mohamed.eskander at 4science.com)
* @author Francesco Bacchelli (francesco.bacchelli at 4science.com)
*/
public abstract class LDNRouterFactory {

View File

@@ -61,22 +61,37 @@ public interface LDNMessageService {
public void update(Context context, LDNMessageEntity ldnMessage) throws SQLException;
/**
* find the oldest queued LDNMessage
* Find the oldest queued LDNMessages that still can be elaborated
*
* @return list of LDN messages
* @param context The DSpace context
* @throws SQLException If something goes wrong in the database
*/
public List<LDNMessageEntity> findOldestMessageToProcess(Context context) throws SQLException;
/**
* find all messages queue timedout and with queue status Processing
* Find all messages in the queue with the Processing status but timed-out
*
* @return all the LDN Messages to be fixed on their queue_ attributes
* @param context The DSpace context
* @throws SQLException If something goes wrong in the database
*/
public List<LDNMessageEntity> findProcessingTimedoutMessages(Context context) throws SQLException;
/**
* Find all messages in the queue with the Processing status but timed-out and modify their queue_status
* considering the queue_attempts
*
* @return number of messages fixed
* @param context The DSpace context
*/
public int checkQueueMessageTimeout(Context context);
/**
* Elaborates the oldest enqueued message
*
* @return number of messages fixed
* @param context The DSpace context
*/
public int extractAndProcessMessageFromQueue(Context context) throws SQLException;
}

View File

@@ -10,8 +10,11 @@ package org.dspace.app.ldn.service.impl;
import java.sql.SQLException;
import java.util.Date;
import java.util.List;
import java.util.Set;
import java.util.UUID;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.gson.Gson;
import com.google.gson.JsonSyntaxException;
import org.apache.commons.lang.time.DateUtils;
@@ -34,7 +37,6 @@ import org.dspace.handle.service.HandleService;
import org.dspace.services.ConfigurationService;
import org.springframework.beans.factory.annotation.Autowired;
/**
* Implementation of {@link LDNMessageService}
*
@@ -81,9 +83,31 @@ public class LDNMessageServiceImpl implements LDNMessageService {
ldnMessage.setOrigin(findNotifyService(context, notification.getOrigin()));
ldnMessage.setTarget(findNotifyService(context, notification.getTarget()));
ldnMessage.setInReplyTo(find(context, notification.getInReplyTo()));
ldnMessage.setMessage(new Gson().toJson(notification));
ObjectMapper mapper = new ObjectMapper();
String message = null;
try {
message = mapper.writeValueAsString(notification);
ldnMessage.setMessage(message);
} catch (JsonProcessingException e) {
log.error("Notification json can't be correctly processed and stored inside the LDN Message Entity");
log.error(e);
}
ldnMessage.setType(StringUtils.joinWith(",", notification.getType()));
Set<String> notificationType = notification.getType();
if (notificationType != null) {
String[] notificationTypeArray = (String[]) notificationType.toArray();
if (notificationTypeArray.length >= 2) {
ldnMessage.setActivityStreamType(notificationTypeArray[0]);
ldnMessage.setCoarNotifyType(notificationTypeArray[1]);
} else {
log.warn("LDN Message from Notification won't be typed because notification has incorrect "
+ "Type attribute");
log.warn(message);
}
} else {
log.warn("LDN Message from Notification won't be typed because notification has incorrect Type attribute");
log.warn(message);
}
ldnMessage.setQueueStatus(LDNMessageEntity.QUEUE_STATUS_QUEUED);
ldnMessage.setQueueTimeout(new Date());

View File

@@ -20,6 +20,8 @@ CREATE TABLE ldn_message
target INTEGER,
inReplyTo VARCHAR(255),
context uuid,
activity_stream_type VARCHAR(255),
coar_notify_type VARCHAR(255),
queue_status INTEGER DEFAULT NULL,
queue_attempts INTEGER DEFAULT 0,
queue_last_start_time TIMESTAMP,

View File

@@ -20,6 +20,8 @@ CREATE TABLE ldn_message
target INTEGER,
inReplyTo VARCHAR(255),
context uuid,
activity_stream_type VARCHAR(255),
coar_notify_type VARCHAR(255),
queue_status INTEGER DEFAULT NULL,
queue_attempts INTEGER DEFAULT 0,
queue_last_start_time TIMESTAMP,

View File

@@ -48,9 +48,8 @@ import org.springframework.web.servlet.config.annotation.ViewControllerRegistry;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
/**
* Define the Spring Boot Application settings itself. This class takes the
* place of a web.xml file, and configures all Filters/Listeners as methods (see
* below).
* Define the Spring Boot Application settings itself. This class takes the place
* of a web.xml file, and configures all Filters/Listeners as methods (see below).
* <p>
* NOTE: Requires a Servlet 3.0 container, e.g. Tomcat 7.0 or above.
* <p>
@@ -100,30 +99,28 @@ public class Application extends SpringBootServletInitializer {
}
/**
* Override the default SpringBootServletInitializer.configure() method, passing
* it this Application class.
* Override the default SpringBootServletInitializer.configure() method,
* passing it this Application class.
* <p>
* This is necessary to allow us to build a deployable WAR, rather than always
* relying on embedded Tomcat.
* This is necessary to allow us to build a deployable WAR, rather than
* always relying on embedded Tomcat.
* <p>
* See:
* http://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#howto-create-a-deployable-war-file
* See: http://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#howto-create-a-deployable-war-file
*
* @param application
* @return
*/
@Override
protected SpringApplicationBuilder configure(SpringApplicationBuilder application) {
// Pass this Application class, and our initializers for DSpace Kernel and
// Configuration
// Pass this Application class, and our initializers for DSpace Kernel and Configuration
// NOTE: Kernel must be initialized before Configuration
return application.sources(Application.class)
.initializers(new DSpaceKernelInitializer(), new DSpaceConfigurationInitializer());
}
/**
* Register the "DSpaceContextListener" so that it is loaded for this
* Application.
* Register the "DSpaceContextListener" so that it is loaded
* for this Application.
*
* @return DSpaceContextListener
*/
@@ -135,8 +132,8 @@ public class Application extends SpringBootServletInitializer {
}
/**
* Register the DSpaceWebappServletFilter, which initializes the DSpace
* RequestService / SessionService
* Register the DSpaceWebappServletFilter, which initializes the
* DSpace RequestService / SessionService
*
* @return DSpaceWebappServletFilter
*/
@@ -185,15 +182,13 @@ public class Application extends SpringBootServletInitializer {
return new WebMvcConfigurer() {
/**
* Create a custom CORS mapping for the DSpace REST API (/api/ paths), based on
* configured allowed origins.
* Create a custom CORS mapping for the DSpace REST API (/api/ paths), based on configured allowed origins.
* @param registry CorsRegistry
*/
@Override
public void addCorsMappings(@NonNull CorsRegistry registry) {
// Get allowed origins for api and iiif endpoints.
// The actuator endpoints are configured using management.endpoints.web.cors.*
// properties
// The actuator endpoints are configured using management.endpoints.web.cors.* properties
String[] corsAllowedOrigins = configuration
.getCorsAllowedOrigins(configuration.getCorsAllowedOriginsConfig());
String[] iiifAllowedOrigins = configuration
@@ -206,48 +201,39 @@ public class Application extends SpringBootServletInitializer {
boolean signpostingAllowCredentials = configuration.getSignpostingAllowCredentials();
if (corsAllowedOrigins != null) {
registry.addMapping("/api/**").allowedMethods(CorsConfiguration.ALL)
// Set Access-Control-Allow-Credentials to "true" and specify which origins are
// valid
// Set Access-Control-Allow-Credentials to "true" and specify which origins are valid
// for our Access-Control-Allow-Origin header
// for our Access-Control-Allow-Origin header
.allowCredentials(corsAllowCredentials).allowedOrigins(corsAllowedOrigins)
// Allow list of request preflight headers allowed to be sent to us from the
// client
// Allow list of request preflight headers allowed to be sent to us from the client
.allowedHeaders("Accept", "Authorization", "Content-Type", "Origin", "X-On-Behalf-Of",
"X-Requested-With", "X-XSRF-TOKEN", "X-CORRELATION-ID", "X-REFERRER",
"x-recaptcha-token")
// Allow list of response headers allowed to be sent by us (the server) to the
// client
// Allow list of response headers allowed to be sent by us (the server) to the client
.exposedHeaders("Authorization", "DSPACE-XSRF-TOKEN", "Location", "WWW-Authenticate");
}
if (iiifAllowedOrigins != null) {
registry.addMapping("/iiif/**").allowedMethods(CorsConfiguration.ALL)
// Set Access-Control-Allow-Credentials to "true" and specify which origins are
// valid
// Set Access-Control-Allow-Credentials to "true" and specify which origins are valid
// for our Access-Control-Allow-Origin header
.allowCredentials(iiifAllowCredentials).allowedOrigins(iiifAllowedOrigins)
// Allow list of request preflight headers allowed to be sent to us from the
// client
// Allow list of request preflight headers allowed to be sent to us from the client
.allowedHeaders("Accept", "Authorization", "Content-Type", "Origin", "X-On-Behalf-Of",
"X-Requested-With", "X-XSRF-TOKEN", "X-CORRELATION-ID", "X-REFERRER",
"x-recaptcha-token")
// Allow list of response headers allowed to be sent by us (the server) to the
// client
// Allow list of response headers allowed to be sent by us (the server) to the client
.exposedHeaders("Authorization", "DSPACE-XSRF-TOKEN", "Location", "WWW-Authenticate");
}
if (signpostingAllowedOrigins != null) {
registry.addMapping("/signposting/**").allowedMethods(CorsConfiguration.ALL)
// Set Access-Control-Allow-Credentials to "true" and specify which origins are
// valid
// Set Access-Control-Allow-Credentials to "true" and specify which origins are valid
// for our Access-Control-Allow-Origin header
.allowCredentials(signpostingAllowCredentials).allowedOrigins(signpostingAllowedOrigins)
// Allow list of request preflight headers allowed to be sent to us from the
// client
// Allow list of request preflight headers allowed to be sent to us from the client
.allowedHeaders("Accept", "Authorization", "Content-Type", "Origin", "X-On-Behalf-Of",
"X-Requested-With", "X-XSRF-TOKEN", "X-CORRELATION-ID", "X-REFERRER",
"x-recaptcha-token", "access-control-allow-headers")
// Allow list of response headers allowed to be sent by us (the server) to the
// client
// Allow list of response headers allowed to be sent by us (the server) to the client
.exposedHeaders("Authorization", "DSPACE-XSRF-TOKEN", "Location", "WWW-Authenticate");
}
}
@@ -263,15 +249,14 @@ public class Application extends SpringBootServletInitializer {
}
/**
* Add a new ResourceHandler to allow us to use WebJars.org to pull in web
* dependencies dynamically for HAL Browser, etc.
* Add a new ResourceHandler to allow us to use WebJars.org to pull in web dependencies
* dynamically for HAL Browser, etc.
* @param registry ResourceHandlerRegistry
*/
@Override
public void addResourceHandlers(ResourceHandlerRegistry registry) {
// First, "mount" the Hal Browser resources at the /browser path
// NOTE: the hal-browser directory uses the version of the Hal browser, so this
// needs to be synced
// NOTE: the hal-browser directory uses the version of the Hal browser, so this needs to be synced
// with the org.webjars.hal-browser version in the POM
registry
.addResourceHandler("/browser/**")

View File

@@ -13,7 +13,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
import java.io.InputStream;
import java.nio.charset.Charset;
import com.google.gson.Gson;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.io.IOUtils;
import org.dspace.app.ldn.model.Notification;
import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
@@ -45,8 +45,10 @@ public class LDNInboxControllerIT extends AbstractControllerIntegrationTest {
context.restoreAuthSystemState();
InputStream offerEndorsementStream = getClass().getResourceAsStream("ldn_offer_endorsement_object.json");
String offerEndorsementJson = IOUtils.toString(offerEndorsementStream, Charset.defaultCharset());
offerEndorsementStream.close();
String message = offerEndorsementJson.replace("<<object>>", object);
Notification notification = new Gson().fromJson(message, Notification.class);
ObjectMapper mapper = new ObjectMapper();
Notification notification = mapper.readValue(message, Notification.class);
getClient(getAuthToken(admin.getEmail(), password))
.perform(post("/ldn/inbox")
@@ -60,7 +62,9 @@ public class LDNInboxControllerIT extends AbstractControllerIntegrationTest {
InputStream announceEndorsementStream = getClass().getResourceAsStream("ldn_announce_endorsement.json");
String message = IOUtils.toString(announceEndorsementStream, Charset.defaultCharset());
Notification notification = new Gson().fromJson(message, Notification.class);
announceEndorsementStream.close();
ObjectMapper mapper = new ObjectMapper();
Notification notification = mapper.readValue(announceEndorsementStream, Notification.class);
getClient(getAuthToken(admin.getEmail(), password))
.perform(post("/ldn/inbox")
.contentType("application/ld+json")
@@ -71,9 +75,11 @@ public class LDNInboxControllerIT extends AbstractControllerIntegrationTest {
@Test
public void ldnInboxEndorsementActionBadRequestTest() throws Exception {
// id is not an uri
InputStream announceEndorsementStream = getClass().getResourceAsStream("ldn_offer_endorsement_badrequest.json");
String message = IOUtils.toString(announceEndorsementStream, Charset.defaultCharset());
Notification notification = new Gson().fromJson(message, Notification.class);
InputStream offerEndorsementStream = getClass().getResourceAsStream("ldn_offer_endorsement_badrequest.json");
String message = IOUtils.toString(offerEndorsementStream, Charset.defaultCharset());
offerEndorsementStream.close();
ObjectMapper mapper = new ObjectMapper();
Notification notification = mapper.readValue(offerEndorsementStream, Notification.class);
getClient(getAuthToken(admin.getEmail(), password))
.perform(post("/ldn/inbox")
.contentType("application/ld+json")

View File

@@ -29,10 +29,16 @@ service.dev-hdc3b.lib.harvard.edu/api/inbox.key = 8df0c72a-56b5-44ef-b1c0-b4dbcb
service.dev-hdc3b.lib.harvard.edu/api/inbox.key.header = X-Dataverse-key
# LDN Queue extractor elaborates LDN Message entities of the queue
ldn.queue.extractor.cron = 0 0/1 * 1/1 * ?
# LDN Queue timeout checks LDN Message Entities relation with the queue
ldn.queue.timeout.checker.cron = 0 0/1 * 1/1 * ?
# LDN Queue extractor elaborates LDN Message entities with max_attempts < than ldn.processor.max.attempts
ldn.processor.max.attempts = 5
# LDN Queue extractor sets LDN Message Entity queue_timeout property every time it tryies a new elaboration
# of the message. LDN Message with a future queue_timeout is not elaborated. This property is used to calculateas:
# a new timeout, such as: new_timeout = now + ldn.processor.queue.msg.timeout (in minutes)
ldn.processor.queue.msg.timeout = 60