Bugfix KNOWAGE-8149 #883
Bugfix KNOWAGE-8149 #883
Conversation
knowage-api/src/main/java/it/eng/knowage/knowageapi/resource/FunctionCatalogResource.java ([53]), knowage-core/src/main/java/it/eng/knowage/backendservices/rest/widgets/PythonProxy.java ([47]), knowage-core/src/main/java/it/eng/spagobi/api/AbstractDocumentResource.java ([49]), knowage-core/src/main/java/it/eng/spagobi/api/BusinessModelResource.java ([104]), knowage-core/src/main/java/it/eng/spagobi/api/common/AbstractDataSetResource.java ([128]),
Added annotations.
Removed annotations from private methods
Updated abstract classes
There was a problem hiding this comment.
I would say that this is a false positive too. The @get, @post and @put as all other HTTP methods annotations like PUT, PATH, DELETE, etc... don't make much sense when the class misses the @path annotation. The SimpleRestClient is just an utility class to let BE makes REST call to other services: it's not its self a REST service. I would ask you to commit only the formatted code.
There was a problem hiding this comment.
Nothing to say, just a check: is the Eclipse formatter correctly set up or it was an already unformatted class?
There was a problem hiding this comment.
We set the Eclipse formatting with Davide Z., it should be exactly the same as you use it, but a lot of the (older) files are unformatted. This is a common issue when creating pull requests and looking for changes.
| @@ -1048,7 +1048,7 @@ public String getDataSet(String label) { | |||
| } | |||
| } | |||
|
|
|||
| public Response deleteDataset(String label) { | |||
| protected Response deleteDataset(String label) { | |||
There was a problem hiding this comment.
Do you think it's possible to use private? If there is no inheritance I would prefer the private modifier.
There was a problem hiding this comment.
This one can't be private, it is being used in DataSetResource.java (two files, one of them is v2).
| @@ -1121,7 +1121,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO | |||
| } | |||
| } | |||
|
|
|||
| public Response execute(String label, String body) { | |||
| protected Response execute(String label, String body) { | |||
There was a problem hiding this comment.
Do you think it's possible to use private? If there is no inheritance I would prefer the private modifier.
There was a problem hiding this comment.
This one can't be private, it is being used in DataSetResource.java (two files, one of them is v2).
| @@ -138,7 +139,7 @@ public Response delete(@PathParam("id") UUID id) { | |||
| } | |||
| } | |||
|
|
|||
| @PATCH | |||
| @PUT | |||
There was a problem hiding this comment.
Is it bug fixing? :-D Just kidding, great job.
There was a problem hiding this comment.
Thank you :)
| @@ -50,7 +50,7 @@ public abstract class AbstractDocumentResource extends AbstractSpagoBIResource { | |||
|
|
|||
| private static final Logger LOGGER = LogManager.getLogger(AbstractDocumentResource.class); | |||
|
|
|||
| public Response insertDocument(String body) { | |||
| protected Response insertDocument(String body) { | |||
There was a problem hiding this comment.
If possible, I would prefer private but this is an abstract class, we need a check.
There was a problem hiding this comment.
This one can't be private, it is being used in DocumentResource.java (two files, one of them is v2).
| @@ -88,7 +88,7 @@ public Response insertDocument(String body) { | |||
| } | |||
| } | |||
|
|
|||
| public Response getDocumentDetails(Object documentIdentifier) { | |||
| protected Response getDocumentDetails(Object documentIdentifier) { | |||
There was a problem hiding this comment.
If possible, I would prefer private but this is an abstract class, we need a check.
There was a problem hiding this comment.
This one can't be private, it is being used in DocumentResource.java (two files, one of them is v2).
| @@ -115,7 +115,7 @@ public Response getDocumentDetails(Object documentIdentifier) { | |||
|
|
|||
| } | |||
|
|
|||
| public Response updateDocument(String label, String body) { | |||
| protected Response updateDocument(String label, String body) { | |||
There was a problem hiding this comment.
If possible, I would prefer private but this is an abstract class, we need a check.
There was a problem hiding this comment.
This one can't be private, it is being used in DocumentResource.java (two files, one of them is v2).
| @@ -133,7 +133,7 @@ public Response updateDocument(String label, String body) { | |||
| return Response.ok().build(); | |||
| } | |||
|
|
|||
| public Response deleteDocument(@PathParam("label") String label) { | |||
| protected Response deleteDocument(@PathParam("label") String label) { | |||
There was a problem hiding this comment.
If possible, I would prefer private but this is an abstract class, we need a check.
There was a problem hiding this comment.
This one can't be private, it is being used in DocumentResource.java (two files, one of them is v2).
| @@ -167,7 +167,7 @@ protected Object getObjectIdentifier(String labelOrId) { | |||
| return documentIdentifier; | |||
| } | |||
|
|
|||
| public Response getDocumentTemplate(String label) { | |||
| protected Response getDocumentTemplate(String label) { | |||
There was a problem hiding this comment.
If possible, I would prefer private but this is an abstract class, we need a check.
There was a problem hiding this comment.
This one can't be private, it is being used in DocumentResource.java (two files, one of them is v2).
| @@ -194,7 +194,7 @@ public Response getDocumentTemplate(String label) { | |||
| return rb.build(); | |||
| } | |||
|
|
|||
| public Response addDocumentTemplate(String label, MultiPartBody input) { | |||
| protected Response addDocumentTemplate(String label, MultiPartBody input) { | |||
There was a problem hiding this comment.
If possible, I would prefer private but this is an abstract class, we need a check.
There was a problem hiding this comment.
This one can't be private, it is being used in DocumentResource.java (two files, one of them is v2).
| @@ -229,7 +229,7 @@ public Response addDocumentTemplate(String label, MultiPartBody input) { | |||
|
|
|||
| } | |||
|
|
|||
| public Response deleteCurrentTemplate(String documentLabel) { | |||
| protected Response deleteCurrentTemplate(String documentLabel) { | |||
There was a problem hiding this comment.
If possible, I would prefer private but this is an abstract class, we need a check.
There was a problem hiding this comment.
This one can't be private, it is being used in DocumentResource.java.
| @@ -257,7 +257,7 @@ public Response deleteCurrentTemplate(String documentLabel) { | |||
| return Response.ok().build(); | |||
| } | |||
|
|
|||
| public Response deleteTemplateById(String documentLabel, Integer templateId) { | |||
| protected Response deleteTemplateById(String documentLabel, Integer templateId) { | |||
There was a problem hiding this comment.
If possible, I would prefer private but this is an abstract class, we need a check.
There was a problem hiding this comment.
This one can't be private, it is being used in DocumentResource.java.
Hello @kerny3d, I reviewed all comments, I don't think any of the abstract methods can be changed from protected to private, they are already being used. I removed the annotations from the knowageutils/../SimpleRestClient.java and just left the formatted code. |
Bugfix KNOWAGE-8149
Files changed:
Possible false positives:
java