access4learning / sif3-framework-java Goto Github PK

Develop Branch
The class hierarchy of this provider class needs to be reviewed and potentially changed once the SIF 3.2 Specification has provided clarification on what operations are available for the Job, Phase and State. This is a related issue of #14.

When I ran the script 'SIF3InfrastructureERM_DDL_mysql.sql' I got the following error:

CREATE INDEX SEC_TOKEN_IDX ON SIF3_SESSION (SECURITY_TOKEN ASC, ADAPTER_TYPE ASC)
Error Code: 1071. Specified key was too long; max key length is 767 bytes

I removed the statement and the rest of the script ran fine.

At the moment in the properties file of a provider in a brokered environment you must supply one of :
env.connector.url
env.connector.url.secure

As far as I'm aware these properties are not used (or needed) for a provider in a brokered environment.

Ubuntu 14.04.3 / Oracle JDK 1.8.0_60

SIF3InfraREST/sif3Common/src/sif3/common/interfaces/QueryConsumer.java:69

* @return A list of responses with one response for each zone/context combination. The data in the response is a �collection-type� style

SIF3InfraREST/sif3Common/src/sif3/common/interfaces/QueryConsumer.java:104

* @return A list of responses with one response for each zone/context combination. The data in the response is a �collection-type� style

In the rows 236-238 of the root pom.xml file, modules are referred to as 'SIF3Common', 'SIF3InfraModel', 'SIF3InfraCommon'. However, those folders are named as 'sif3Common', 'sif3InfraModel', 'sif3InfraCommon' (note 'sif' instead of 'SIF').

<modules>
	<module>SIF3Common</module>
	<module>SIF3InfraModel</module>
	<module>SIF3InfraCommon</module>
	<module>SIF3REST</module>
</modules>

The above lines should be written as:

<modules>
	<module>sif3Common</module>
	<module>sif3InfraModel</module>
	<module>sif3InfraCommon</module>
	<module>SIF3REST</module>
</modules>

When we receive an error response from a delayed consumer call we are always getting a 500 error : An error occurred unmarshalling object from XML: java.lang.NullPointerException.

I believe this is because in RemoteMessageQueueReader.processError the ErrorInfo object is built by calling setResponseBaseData(errorInfo, response, MessageType.ERROR);

setResponseBaseData populates the payload of the errorInfo object from response.getDataObject when in reality it should be populated from response.getError

Our response looks like this :

Response [dataObjectType=class java.lang.String, dataObject=null, resourceURI=<url>, toString()=BaseResponse [status=200, statusMessage=OK, hasEntity=true, error=ErrorDetails [description=The provider response exceeded the timeout of 60 seconds., errorCode=504, message=<error>, scope=PROVIDER], hdrProperties=HeaderProperties [hdrProperties={date=Tue, 10 Jul 2018 03:59:59 GMT, responsesource=PROVIDER, server=Apache-Coyote/1.1, content-length=351, servicetype=OBJECT, messagetype=ERROR, vary=Accept-Encoding, requestid=<id>, messageid=<id>, relativeservicepath=<servicepath>, connection=keep-alive, content-type=application/xml;charset=UTF-8}], mediaType=application/xml;charset=UTF-8, contentLength=351, zone=null, context=null, delayedReceipt=null]]

As you can see dataObject is null. I am not sure if this is pointing to an error further up in the framework (perhaps BaseClient.setResponse) where the error payload is not being set as the dataObject or if this is the intended behaviour of BaseClient.setResponse when an error is received.

I was updating my implementation of the framework from version 0.12.0 to 0.13.0 using the script found here. However, MySQL reported the following error:

0 | 6 | 09:47:15 | CREATE INDEX JOB_FINGERPRT_ADPTYPE_IDX ON SIF3_JOB (FINGERPRINT ASC, SERVICE_NAME ASC, ADAPTER_TYPE ASC) | Error Code: 1071. Specified key was too long; max key length is 767 bytes | 0.000 sec -- | -- | -- | -- | -- | --

I would assume that this applies to many other indexes in the statement, but I have yet to try those yet.

Also, according to the MySQL documentation the max length for an index name is 64 characters. However, none of these names are over 64 characters, so I think it has something to do with the encoding type, which is utf8.

If you attempt to indicate mime type by adding an extension onto your request, but you are also using matrix parameters then the framework fails to detect the mime type.

This is due to setURLMediaTypeFromPath in BaseResource including any matrix parameters when attempting to parse the mime type. This issue is exacerbated when your matrix parameters contain "."

eg.
http://sif-server.com/requests/StudentPersonals/31b056ae-8178-4348-97c2-43be3ea52c95.json;zoneId=urn:com.sif-provider:school:1234

would try and parse
sif-provider:school:1234 as the Mime Type

In file SIF3Infra.hbm.xml:

<property name="environmentXML" column="ENVIRONMENT_XML" type="string"/>

In PostgreSQL this maps to a varchar(255) which is too small for most environment xmls. Changing the type to "text" makes everything awesome.

Develop Branch
As with issue #14 & #15 there might be a need to change the hierarchy of these interfaces. This cannot be done until the SIF 3.2 Specification has provided clarity on the exact functionality for Jobs, Phases and States.

This issue has 2 components.

1. DataModelResource currently returns 201 created as the status code for successfully received create many requests. From the 3.2.1 specification create many should return

An HTTP Response with a status of 200 and a payload consisting of a single “createResponse” element.

2. ObjectServiceClient currently considers a 200 response to be an error response. This results in individual status codes being discarded.

Currently when populating the Timestamp header the Framework uses ISO formatted local time with 'Z' appended to indicate GMT time. This means that the framework generates timestamps in the future or past depending on your local timezone eg.
At 2:21pm in Perth Australia on the 27th of November the Timestamp being generated is:
2017-11-27T14:21:00Z

the actual timestamp should be :
2017-11-27T06:21:00Z

This issue hosts the images for the wiki associated with this repository:

Develop Branch
This consumer extends the AbstarctConsumer. The AbstractConsumer is really geared towards Object Services. I cannot see the AbstractFunctionalServiceConsumer extending the AnstractConsumer at the moment. Through that extension many methods that are only applicable for Object Consumers are suddenly part of the Functional Services. For example all "Delayed" functionality is not applicable for the Job Object. It might be applicable for phases but not for the actual Job. Further all "Batch" operations are not applicable for the Job Object either. I think we need to build a slightly different hierarchy within the sif3.infra.rest.consumer package, so that you can leverage existing functionality. I can think of something like a BaseSingleCRUDConsumer, AbstractConsumer (extending the BaseSingleCRUDConsumer) and adding the batch and delayed functionality, AbstractEventConsumer (extending the AbstractConsumer) adding the event stuff, AbstractFunctionalServiceConsumer (extending the BaseSingleCRUDConsumer) and implementing the FunctionalServiceConsumer interface etc. This is mainly a refactoring exercise and pulling apart the current Abstract Consumers. This way your FunctionalServiceConsumer does not have to deal with the Delayed and Batch operations which is matching the SIF Specification. It would also allow you to implement this without the Event functionality. An AbstractEventFunctionalConsumer could come in a second step.
Note: The current SIF 3.2 Specification does not clearly indicate the suggested hierarchy and related functionality. This issue needs to be reviewed once the SIF Specification has clearly stated the functionality for a Job, Phase and State.

Currently it is possible both in RemoteMessageQueueReader and LocalMessageConsumer for the framework to encounter an exception and then fall out of the while(true) loop that the framework is using to read the queues.

This can be confusing because the rest of your consumer can be working fine however it can get into a state where it is no longer checking the remote queue and or the local queue.

When shutting down my consumers using ConsumerLoader.shutdown() none of the threads that poll the remote message queues are actually shut down. They continue to run until the application server is shutdown, even though they start failing because they no longer have access to the consumer credentials.

This is because the ConsumerLoader calls service.shutdown() which interrupts idle threads and waits for currently executing threads to become idle before executing them. The way the message readers are implemented using a while (true) loop means that they never become idle.

I believe if you change the call to be service.shutdownNow() this will interrupt currently executing threads and will fix this issue.