Joe's Review #2

[jgrassel]

Reviewing https://qa-guides.mybluemix.net/guides/jpa-intro.html in its current state.

Section What you’ll learn:

"The JPA specification is implemented by several different persistence providers."

This reads a little awkward to me.

"In this guide you will use EclipseLink as the persistence provider because it is the implementation that is shipped with Liberty."

We actually ship 2 JPA provider implementations, Apache OpenJPA for the jpa-2.0 feature, and Eclipselink for the jpa-2.1 and jpa-2.2 (and expectingly all future) features.

"JPA simplifies object-relational mapping (ORM) by using annotations and/or XML to map Java objects to tables in a relational database."

As a newcomer, I'd wonder what they mean about "XML". I'd say "Object Relational Mapping XML Documents" or something to that effect.

"JPA also reduces the burden of having to write JDBC and SQL code when performing database operations."

True, but I think it is also worthwhile to figure a way to mention that JPA also takes care of database vendor specific differences in (most) cases. It doesn't get everything, but it does a fairly decent job. Anyone who has written a JDBC application that supports multiple database vendors is already familiar with having to write complicated if-blocks addressing database vendor differences with trying to accomplish the same thing.

Section Defining a JPA entity class

"A JPA entity is a Java object whose non-transient fields will be persisted to the database."

I'd say non-transient and non-static fields. Static fields are not considered persistent fields.

"JPA uses a database table for every entity and persisted instances will be represented as one row in the table."

Maybe say JPA maps an entity type to a database table (although you can also map an entity to secondary tables.) But I think "map" is a better word than "uses" here.

This block here:

<logging  traceSpecification="eclipselink.sql=all"
  traceFileName="trace.log"
  maxFileSize="20"
  maxFiles="10"
  traceFormat="BASIC" />

I don't think "eclipselink.sql=all" is a valid trace spec. I think you want "JPA=all". That will trace both the JPA Runtime Integration as well as integrate Eclipselink's own logging into Liberty's trace system.

Section Performing CRUD operations using JPA:

Please say "EntityManager" and not "entity manager".

"Lastly, removed entities are ones that have been removed from the database."

Removed entities are marked for removal, the DELETE FROM TABLE won't be issued to the database until flush or transaction commit.

[yeekangc]

I have been wondering about the portion regarding the persistence provider too. It doesn't appear that they add to the guide or they are relevant to the focus of the guide. I suggest that they be removed. It shouldn't matter to a developer getting going with JPA what we use under the cover.

The logging stanza in the server.xml should be removed. Guess we missed that during code reviews. It's not needed and adds clutter.