-
Notifications
You must be signed in to change notification settings - Fork 17
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
Feature/nta restapi #68
base: main
Are you sure you want to change the base?
Conversation
Modified nta-dis pom.xml to remove the webapp module Modified restapi pom.xml to downgrade the rest api version to fix the issue with CDI injection Removed IHelperService and implementation. Added new Transaction Service interface
Updated the main pom file and the nta-dist pom file.
Added a URI Constant class to hold the uri constants for API's.
Added a transaction info model object to map the transaction model object from core. TransactionInfo object returns collection of links for collections instead of actual objects. Added TransactionService interface and implementation. Added a LinkBuilder helper to help build REST links for collections.
… Feature/NTA_Restapi Conflicts: pom.xml
Merged master into current branch. Added a new Tracer API Added a new TraceLogging interface and implementation to enable and disable tracing. Changed the location of URI Constants
Updated TracerAPI with the methods to call the TracerService. Removed unused method from TracerService interface and implementation Used media type for Produces annotation instead of string.
Added Manifest file for Client Controller Added validation check for Transaction to check if parent transaction is null.
Added new Exception Handlers that implements ExceptionMappers Modified the LinkBuilder class to provide the correct url for participant record Added new method to get transaction by id for the API and service classes Added hibernate validation on the requests for the api.
Removed wrong exclamation mark.
Added new response entities. Removed NTA_URI and updated the LinkBuilder. Updated the nta-dist/pom.xml
Started testing this pull request: http://172.17.131.2/job/pulls-transaction-analyser/32/ |
@@ -91,6 +96,11 @@ | |||
<webModule> | |||
<groupId>io.narayana.nta</groupId> | |||
<artifactId>webapp</artifactId> | |||
<contextRoot>/ntaweb</contextRoot> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we leave the web-console on /nta? Maybe change the rest api to be on /nta/rest or something? Is there a defacto standard for that url?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will revert the url change made on web-console and update the rest url to nta/rest. There is no defacto standard but the preferred standard is "host/project/rest/api/version/endpoint". This can be seen in the following line: https://github.com/Amila17/transaction-analyser/blob/Feature/NTA_Restapi/restapi/src/main/java/io/narayana/nta/restapi/models/URIConstants.java#L12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so switching the context root to "nta/rest" should fix the problem without breaking your URL? Assuming it works of course. Overlapping context roots might not be allowed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The over lapping context root url might be a problem, I will need to test it to confirm on that.
Can you explain what the uriBuilder is in the response from http://localhost:8080/nta/rest/api/v1/transaction? It appears in the events and participantRecords.
|
@@ -0,0 +1,8 @@ | |||
<web-app version="3.0" xmlns="http://java.sun.com/xml/ns/javaee" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this file needed?
I get an error when visiting: http://localhost:8080/nta/rest/api/v1/participantRecord status: "INTERNAL_SERVER_ERROR", |
Is the API at http://docs.ntarestapi.apiary.io/ still current? I don't see /resourcemanager implemented here? Maybe you haven't got that far yet? |
As mentioned in the PR, only the trace and transaction api is complete. I will also need to update the apiary docs to reflect this. |
Ah yes, sorry. I read the intro a while ago and forgot about it ;-) Do you intend to implement everything in the Apiary.io documented API now, or later? If it's going to be in another release of NTA, maybe you could create a v0.1 API in Apiary.io for this release, and keep the current one for documenting what it will look in the future? |
No worries. |
For the comment on the uriBuilder response. This was intended to return a REST url for nested objects to the client instead of the actual object. For example the participant records within a transaction will be returned as REST urls instead of the actual content. I used the LinkBuilder class to achieve this but the output does not look very neat. May be its better to create a custom model for this purpose and return that model? |
I think what I really mean is, will the full API be in the first release of this work? We agreed that the first release would just replicate what we have today, but using REST and Angular. As for time frames, don't worry about it. We'll put this work in when it's ready (i.e. replicates what we have today). |
WRT the link. I was just expecting a single string value. E.g:
Would that make sense, do you think? But I'm not that experienced in REST APIs, so I don't know what is common practice here. |
The API for the first release will be the same functionality exposed by the web module and the Angular stuff on the frontend. Regarding the link, I think the example you just showed is the best approach. Its much cleaner and clearer. I will do this change and update the PR asap. |
The REST link returned for an entity will be in the following format: |
Added JBoss Copyright message to all files. Updated the file header template. Changed the context of webapp and restapi modules. Discarded use of LinkBuilder class and implemented custom REST link generators. Updated the TransactionInfo Model to adhere to the change of REST links for an entity. Updated the TransactionService to return better response objects to API level. Updated API to return 204 status if no content is found.
@paulrobinson I am using IntelliJ. I believe I have setup my IDE to use a custom formatting setting. I will change this to default settings and re-format the code accordingly. |
Re-formatted code layout to match the format followed by the NTA project.
Started testing this pull request: http://172.17.131.2/job/pulls-transaction-analyser/35/ |
Changed folder naming convention. Added API class for ParticipantRecord entity. Added Serivce class interface and implementation to process the ParticipantRecord REST requests. Updated the Link Generator class to generate links for Participant Records.
Started testing this pull request: http://172.17.131.2/job/pulls-transaction-analyser/36/ |
@Amila17 it looks like you are on the right track. Let us know if you need us to review anything. |
Added named query to Event model. Added two new methods to DataAccessObject to get events and get event by id. Added EventAPI class. Added EventInfo model Added EventService interface and implementation
Started testing this pull request: http://172.17.131.2/job/pulls-transaction-analyser/37/ |
Started testing this pull request: http://172.17.131.2/job/pulls-transaction-analyser/38/ |
… Feature/NTA_Restapi
Moved Get and GetById methods to a generic Common Service interface. Added two specific interfaces for Transaction and ResourceManager which extends Common Service interface. Updated the implementation of the service classes to implement Common Service interface. Updated the REST API to return bad request for a entity that does not exists in the system.
Removed @nAmed parameter from TraceLoggingServiceImpl class.
Started testing this pull request: http://172.17.131.2/job/pulls-transaction-analyser/42/ |
Added the EventAPI class to query event information.
Started testing this pull request: http://172.17.131.2/job/pulls-transaction-analyser/43/ |
Added a TransactionAPI interface and implementation. Interface was added to enable testing so the ProxyFactory can proxy the interface.
Started testing this pull request: http://172.17.131.2/job/pulls-transaction-analyser/44/ |
@Amila17 can you squash all the commits into one ? I'm preparing to merge this PR. Thanks a lot ! |
Started testing this pull request: http://narayanaci1.eng.hst.ams2.redhat.com/job/transaction-analyser-pulls/190/ |
@Amila17 FYI I am closing this PR since the Blacktie project is now archived (https://groups.google.com/g/narayana-users/c/k0EgFlS6VJc/m/6DcliSBfEAAJ). But thanks very much for your PR. What are you using for a transaction manager now? |
Sorry @Amila17 I got the analyser confused with the archived BlackTie project - I have re-opened your PR |
Created new REST API module and updated the POM.xml and nta-dist.pom.xml
Added new API classes(Root, Tracer and Transaction)
Added new service classes (Transaction and TraceLogging)
Added new Exception Handlers that implements ExceptionMappers
Added a LinkBuilder class to provide the correct url for collections within entities.
Added hibernate validation on the requests for the api.
Added Response Classes to create well formed responses from the REST API.
This feature is still a work in progress. However, I would like the current work to be reviewed before moving on to the Participant and Event API classes. In addition, each of the API and Service classes will require unit tests.
Update:
New API classes have been added for ResourceManager and ParticipantRecord entities.
New service classes have been added which corresponds to the above two new API's.
Updated the LinkBuilder to build url encoded api uri's for ResourceManager entities.
Added new queries to Resource Manager entity class and DataAccessObject class.