Move builder methods `withText` and `withXml` to their own traits #6

hosamaly · 2018-03-26T15:43:29Z

Move builder methods withText and withXml to their own traits
Add DocDataBuilder#withDocId(String) and BodyBuilder#withEnd(BodyEnd)
Accept an optional TitleType in HeadBuilder#withTitle
Implement an AbstractBuilder and a MediaCaptionBuilder

These can go into a release, and then a separate release can be made for:

Deprecate MixedContentBuilder#withXml

It wasn't included in the actual `libraryDependencies` so this change has no effect on the dependency structure.

Grouping all builder traits together

…nd)` Additionally, make `DocDataBuilder#withDocDataOption` `protected` in line with other helper methods for easier extensibility. Finally, print the schema version in the `TwoWaySpec` test.

Introduce `MixedContentBuilder` and `AnyContentBuilder` to encapsulate these methods. `MediaReferenceBuilder` now extends `MixedContentBuilder`. `AnyContentBuilder` is not used in this code base but an example in its docs shows how to use it.

It's not safe. The caller can inject any XML that doesn't match the schema. In all builders of mixed-content objects, values are appended to the element. Combining these methods with `withText`, all forms of valid XML can be generated.

hosamaly · 2018-03-26T15:44:32Z

src/main/scala/Builders.scala

@@ -42,6 +43,31 @@ trait Builder[T] {
  override def toString: String = build.toString
 }

+trait BlockContentBuilder {


This class was moved from line 155.

hosamaly · 2018-03-26T15:45:34Z

src/main/scala/Builders.scala

+  def withNitfTable(x: NitfTable): this.type = withBlockContent(x)
+  def withSubordinateHeadline(x: Hl2): this.type = withBlockContent(x)
+
+  def withTextParagraph(x: String, markAsSummary: Boolean = true): this.type = {


This is a new helper method in this class. It used to exist as BodyHeadBuilder#withAbstract(String, Boolean).

hosamaly · 2018-03-26T15:46:05Z

src/main/scala/Builders.scala

@@ -62,26 +88,66 @@ trait EnrichedTextBuilder {
  def withObjectTitle(x: ObjectTitle): this.type = withEnrichedText(x)
  def withPronunciation(x: Pronounce): this.type = withEnrichedText(x)
  def withVirtualLocation(x: Virtloc): this.type = withEnrichedText(x)
-  def withXml(x: NodeSeq): this.type = withContent(dataRecord(x))
-  def withText(x: String): this.type = withContent(dataRecord(x))


This two methods moved into MixedContentBuilder and AnyContentBuilder.

hosamaly · 2018-03-26T15:46:59Z

src/main/scala/Builders.scala

 }

 class BodyHeadBuilder(var build: BodyHead = BodyHead()) extends Builder[BodyHead] {
  def withByline(x: String): this.type = withByline(Byline(Seq(dataRecord(x))))
  def withHeadline(x: String): this.type = withHeadline(new HeadlineBuilder().withPrimaryHeadline(x))
-  def withAbstract(x: NodeSeq): this.type = withAbstract(Abstract(Seq(dataRecord(x))))


This method was removed in line with the deprecation of withXml.

hosamaly · 2018-03-26T15:54:38Z

src/main/scala/Builders.scala

+  *   val xml = scalaxb.toXML(nitf, None, None, toScope(Some("my") -> "http://www.example.com/my-extension"))
+  * }}}
+  */
+trait AnyContentBuilder { this: Builder[_ <: { def any: Seq[scalaxb.DataRecord[Any]] }] =>


Codacy complains about the structural type expression here. However, I don't think there is anything wrong with it. We're not accessing any of the fields of the Builder, and the generic type is erased at runtime anyway.

regiskuckaertz · 2018-03-26T16:04:38Z

src/main/scala/Builders.scala

+  @deprecated("Use multiple invocations of other methods for a type-safe approach", since = "3.x.3")
+  def withXml(x: NodeSeq): this.type = withContent(dataRecord(x))
+
+  protected def withContent(x: DataRecord[_]): this.type


It looks like the implementation is the same every time this trait is extended. Worth providing it as a default implementation?

I wish I could! Because the implementation calls .copy, I don't think it's possible to do this other than with a macro.

Do you know of another way to do it?

I saw it too late 😩 I suppose you could define an abstract class inheriting from Builder and MixedContentBuilder? That may not be very elegant though...

Even if I did this, I still won't be able to implement withContent. That's because case classes' .copy() is not defined in a trait.

regiskuckaertz · 2018-03-26T16:13:41Z

src/test/scala/BuildersSpec.scala

@@ -32,7 +32,7 @@ class BuildersSpec extends FunSpec {
          .withHead(new BodyHeadBuilder()
            .withHeadline("News Article")
            .withByline("It took a lot of work to get there")
-            .withAbstract(<p>It wasn't easy, but they <em>never</em> gave up!</p>)
+            .withAbstract(new AbstractBuilder().withTextParagraph("It wasn't easy, but they never gave up!"))


I suppose you had to remove the em to avoid having to parse the input string. Any chance the input will ever contain tags and/or references?

I removed the <em> because I removed/deprecated the methods that accept XML.

The input can still contain an em, except we'd expect it to be added, type-safely, like this:

.withAbstract(new AbstractBuilder() .withParagraph(new ParagraphBuilder() .withText("It wasn't easy, but they ") .withEmphasis(Em(Seq(dataRecord("never")))) .withText(" gave up!") ) )

In another project, I intend to create a utility that converts Jsoup nodes to NITF nodes so that we can parse HTML and convert it to NITF.

regiskuckaertz

Great as soon as the build passes 👍

hosamaly added 9 commits March 23, 2018 16:33

Remove the reference to an unused library (scalactic)

eb992ce

It wasn't included in the actual `libraryDependencies` so this change has no effect on the dependency structure.

Implement an AbstractBuilder

aac39eb

Move BlockContentBuilder to the top of the Builders file

a7ab1e4

Grouping all builder traits together

Compile with more stringent checks

7d1e470

Accept an optional TitleType in HeadBuilder#withTitle

0883de9

Add DocDataBuilder#withDocId(String) and `BodyBuilder#withEnd(BodyE…

5c0d2d6

…nd)` Additionally, make `DocDataBuilder#withDocDataOption` `protected` in line with other helper methods for easier extensibility. Finally, print the schema version in the `TwoWaySpec` test.

Test scaladoc syntax in Travis CI

83865d9

Deprecate MixedContentBuilder#withXml

74a60d0

It's not safe. The caller can inject any XML that doesn't match the schema. In all builders of mixed-content objects, values are appended to the element. Combining these methods with `withText`, all forms of valid XML can be generated.

hosamaly commented Mar 26, 2018

View reviewed changes

regiskuckaertz reviewed Mar 26, 2018

View reviewed changes

regiskuckaertz approved these changes Mar 26, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move builder methods `withText` and `withXml` to their own traits #6

Move builder methods `withText` and `withXml` to their own traits #6

hosamaly commented Mar 26, 2018 •

edited

Loading

hosamaly Mar 26, 2018

hosamaly Mar 26, 2018

hosamaly Mar 26, 2018

hosamaly Mar 26, 2018 •

edited

Loading

hosamaly Mar 26, 2018

regiskuckaertz Mar 26, 2018

hosamaly Mar 26, 2018

regiskuckaertz Mar 26, 2018

hosamaly Mar 26, 2018

regiskuckaertz Mar 26, 2018

hosamaly Mar 26, 2018

regiskuckaertz left a comment

Move builder methods withText and withXml to their own traits #6

Are you sure you want to change the base?

Move builder methods withText and withXml to their own traits #6

Conversation

hosamaly commented Mar 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hosamaly Mar 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

regiskuckaertz left a comment

Choose a reason for hiding this comment

Move builder methods `withText` and `withXml` to their own traits #6

Move builder methods `withText` and `withXml` to their own traits #6

hosamaly commented Mar 26, 2018 •

edited

Loading

hosamaly Mar 26, 2018 •

edited

Loading