We need your help to get OpenMRS running Java 8

wyclif · May 8, 2015, 8:37pm

Hi,

We managed to get master to compile and run against java8, apparently our CI plan that generates the javadocs is broken because in java 8 they added very strict rules to validate the javadocs, Some can be suppressed but it seems like some we need to fix, if you know how to suppress all please let us know otherwise we are looking for someone to help us clean up our javadocs so that they conform to the strict rules. Below is what you need to execute from the root of the openmrs project to generate the javadocs:

mvn package javadoc:aggregate-jar -DskipTests

NOTE: this applies to TRUNK-4583.

surangak · May 9, 2015, 5:12am

Hi @wyclif,

Umm… I think I may have misunderstood, but as I read your email, you have two specific wants… is that right?

(a) Someone needs to figure out how to suppress validation for java8 Or if that fails, (b) volunteers need to go over each failure and edit the java docs in the master so that they work.

Is this interpretation right?

burke · May 11, 2015, 3:34pm

@wyclif, could you provide some specifics on what types of problems there are?

wyclif · May 11, 2015, 7:37pm

That’s correct @surangak, the problems that are reported are:

self-closing element not allowed e.g <br/>, i already fixed these by adding something to the configuration of the maven-javadoc-plugin
bad use of ‘>’ e.g <String, PersonAttribute>, the angel brackets need to be escaped
malformed HTML e.g having a stray ‘<’ javadocs, such need to be removed
@return tag has no arguments, you are required to add something about the return value like a description
@see: reference not found, this is because the referenced class or method or field in the javadocs doesn’t exist
@see:illegal character, reference with this tag have to be pointing to something like a class and not something else like an xml file
@throws tag has no arguments
Missing closing ‘}’ character for inline tag: “{@link #updateConceptIndex(Concept)”
Missing @link: reference not found, the referenced class or method or field in the javadocs doesn’t exist
Using an invalid name with the @param annotated, probably it was removed
Using unknown annotations and tags in the javadocs
Using @Deprecated instead of @deprecated in javadocs

burke · May 12, 2015, 1:44am

This is bogus, but a s/<br\s*\/>/<br>/g throughout the codebase should solve it.

This is fair as long, since javadocs use HTML. All literal less than & greater than symbols should be written as < and >.

These all seem fair validation checks for lazy or lousy documentation on our part that should be cleaned up.

wyclif · May 12, 2015, 1:55am

Majority of them are fair, the self closing one is the annoying one since it is legal HTML but the good thing is that it has a work around

surangak · May 12, 2015, 2:57am

Looks like these would make some wonderful intro tickets some day. Easy to fix, but legion in number

wyclif · May 12, 2015, 4:09am

Yeah, but we need this fixed ASAP to have the CI plan back to green

raff · May 12, 2015, 8:47am

According to this article http://blog.joda.org/2014/02/turning-off-doclint-in-jdk-8-javadoc.html we can simply switch off validation temporarily with

<plugins>
    <plugin>
      <groupId>org.apache.maven.plugins</groupId>
      <artifactId>maven-javadoc-plugin</artifactId>
      <configuration>
        <additionalparam>-Xdoclint:none</additionalparam>
      </configuration>
    </plugin>
  </plugins>

That said we should still create issues in JIRA for all mentioned errors and fix them to eventually turn it back on.

wyclif · May 12, 2015, 7:16pm

@raff from my posts here, i did mention that i already set that but it doesn’t turn of all validation, it only suppressed the one for self enclosing tags

raff · May 13, 2015, 3:18pm

I looked into that and after adding -Xdoclint:none the validation produces warnings instead of errors, which is ok.

The build failed due to a different error:

javadoc: error - com.sun.tools.doclets.internal.toolkit.util.DocletAbortException: com.sun.tools.javac.code.Symbol$CompletionFailure: class file for com.fasterxml.jackson.databind.cfg.HandlerInstantiator not found

I noticed that the javadoc plugin included for analysis some packages from org.springframework that lead to that error. I’m not sure why it did that, but I excluded them and the build went green. See https://ci.openmrs.org/browse/JAVADOC-TRUNK-484

wyclif · May 13, 2015, 5:55pm

Thanks! i created TRUNK-4711 to follow up on the warning and clean the javadocs

kosmik · June 29, 2015, 3:16pm

Dear team,

I have been working on TRUNK-4711 and thought I should bring my findings to a wider attention.

In a nutshell, I have fixed most of the javadoc issues but there are still over 30 issues I feel should be looked at by a more experienced developer. Basically most of these issues seem to be related to refactorings, where an implementation has been renamed or removed but javadocs referencing said implementation have not been updated. Here is the list:

Cohort.java:36: warning - Tag @link: reference not found: ReportService#evaluate(org.openmrs.report.ReportSchema, Cohort, EvaluationContext)
Cohort.java:36: warning - Tag @see: reference not found: org.openmrs.cohort.CohortDefinition
Hl7InArchivesMigrateThread.java:29: warning - Tag @see: can't find startHl7ArchiveMigration() in org.openmrs.hl7.HL7Service
HL7Service.java:574: warning - Tag @see: can't find processHL7String(String) in org.openmrs.hl7.HL7Service
HL7ServiceImpl.java:254: warning - Tag @see: can't find getHL7InQueueBatch(int, int, java.lang.String) in org.openmrs.hl7.HL7Service
CustomDatatypeUtil.java:144: warning - Tag @link: reference not found: AttributeHandler
CustomDatatypeUtil.java:122: warning - Tag @link: reference not found: AttributeHandler
CustomDatatypeUtil.java:144: warning - Tag @link: reference not found: AttributeHandler
Attribute.java:31: warning - Tag @see: reference not found: AttributeHandler
AttributeType.java:26: warning - Tag @see: reference not found: AttributeHandler
Operand.java:27: warning - Tag @link: reference not found: OperandDate
Operand.java:27: warning - Tag @link: reference not found: OperandConcept
Operand.java:27: warning - Tag @link: reference not found: OperandDate
Operand.java:27: warning - Tag @link: reference not found: OperandConcept
DatabaseUpdater.java:93: warning - Tag @see: can't find update(Map) in org.openmrs.util.DatabaseUpdater
ConceptInUseException.java:19: warning - Tag @see: can't find checkIfConceptIsAttachedToAnObservation() in org.openmrs.api.ConceptService
ObsService.java:50: warning - Tag @see: reference not found: org.openmrs.ComplexObs
Context.java:828: warning - Tag @link: can't find update(Map) in org.openmrs.util.DatabaseUpdater
Context.java:874: warning - Tag @link: can't find update(Map) in org.openmrs.util.DatabaseUpdater
EncounterVisitHandler.java:27: warning - Tag @see: can't find getVisitAssignmentHandlers() in org.openmrs.api.EncounterService
FormEntryHandler.java:34: warning - Tag @link: can't find getFormList(FormEntryContext) in org.openmrs.module.web.extension.FormEntryHandler
StaticDispatcherServlet.java:29: warning - Tag @see: can't find reInitFrameworkServlet() in org.openmrs.web.StaticDispatcherServlet
DateFieldGenDatatypeHandler.java:30: warning - Tag @link: reference not found: DateAttributeHandler
ShortPatientFormValidator.java:39: warning - Tag @link: reference not found: NewPatientFormController
WebBinaryDataHandler.java:54: warning - Tag @see: can't find getComplexData(org.openmrs.Obs, java.lang.String) in org.openmrs.obs.handler.BinaryDataHandler
WebBinaryStreamHandler.java:54: warning - Tag @see: can't find getComplexData(org.openmrs.Obs, java.lang.String) in org.openmrs.obs.handler.BinaryStreamHandler
WebImageHandler.java:57: warning - Tag @see: can't find getComplexData(org.openmrs.Obs, java.lang.String) in org.openmrs.obs.handler.ImageHandler
WebTextHandler.java:56: warning - Tag @see: can't find getComplexData(org.openmrs.Obs, java.lang.String) in org.openmrs.obs.handler.TextHandler
DateFieldGenDatatypeHandler.java:30: warning - Tag @link: reference not found: DateAttributeHandler
ShortPatientFormValidator.java:39: warning - Tag @link: reference not found: NewPatientFormController
DateFieldGenDatatypeHandler.java:30: warning - Tag @link: reference not found: DateAttributeHandler
DateFieldGenDatatypeHandler.java:30: warning - Tag @link: reference not found: DateAttributeHandler
DateFieldGenDatatypeHandler.java:30: warning - Tag @link: reference not found: DateAttributeHandler
DateFieldGenDatatypeHandler.java:30: warning - Tag @link: reference not found: DateAttributeHandler
CustomDatatypeUtil.java:144: warning - Tag @link: reference not found: AttributeHandler
ShortPatientFormValidator.java:39: warning - Tag @link: reference not found: NewPatientFormController

wyclif · June 29, 2015, 11:56pm

I commented on the ticket

kosmik · July 2, 2015, 11:40am

While fixing the existing javadoc comments, I started wondering how are we going to arrange things so that developers get all the support they need for writing valid javadocs? Maybe this problem has already been solved but I just wanted to ask to be sure.

I’m not yet very familiar with the CI process here and was wondering what is the visibility of javadoc compilation errors to your average OpenMRS developer? For example, if a developer implements some changes, does the pull request etc, when and how will said committer get notified that her modifications caused a javadoc compilation error?

Ideally the developer should see javadoc errors a) immediately in the IDE or b) latest at “mvn install”. Regarding a), I gave Eclipse’s javadoc validation a try (see Preferences, Java, Compiler, Javadoc) but could not get it to use the same validation logic as “javadoc:aggregate-jar” does. Regarding b), I guess “mvn install” is not hooked up to javadoc validation at the moment but maybe it should?

To give you some perspective, based on the number and type of javadoc errors I have been fixing in TRUNK-4711, I would be as bold as to say there is not a single person on this project who can write valid javadocs without feedback from the validator. And that also includes yours truly!

wyclif · July 6, 2015, 7:03pm

Hi Mikko,

I agree with you that it would be nice to have the javadoc validation done when building the project. I think one of the reasons it isn’t currently is to keep the build time low.

cintiadr · July 6, 2015, 11:53pm

Is it a problem for the core only or all the modules?

wyclif · July 7, 2015, 8:32pm

Core is the only one we made changes to build against java 8, so yes the problem only shows up for core and not modules