[jira] [Comment Edited] (PDFBOX-2580) Decouple implementation specific forms handling from interactive.form PD Model

["John Hewson (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/PDFBOX-2580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14310925#comment-14310925 ] 

John Hewson edited comment on PDFBOX-2580 at 2/7/15 8:50 PM:
-------------------------------------------------------------

Thanks for the detailed explanation. The changes which you're proposing to the forms API are pretty big and the specifics need to be proposed in more detail before we have something which everyone is happy with.

{quote}
The next step - and this is one of the reasons why the .services.forms package has been created - is to further enhance the usability of forms filling and creation. As of today creating a new form field users need to know that they have to create a field, have to create an annotation, set the annotation to be used within the field, apply styling to the annotation .... - not very user friendly.
{quote}

That's a very good goal but the place to add this functionality is in the PD model, as that where our high-level code lives., e.g. the new font embedding features are very sophisticated and are entirely implemented within the PD model. Convenience methods for adding form fields can be added to PDPage, perhaps by replacing List<PDAnnotation> returned by getAnnotations() with a PDAnnotations (note the 's') object which implements List<PDAnnotation> but also provides all the necessary helper methods for creating new annotations, including form fields. Alternatively there may be other designs such as having a PDFormFields getFormFields() method on PDDocument, where PDFormFields performs the high-level convince functions - whatever is the most appropriate design. Can you suggest some appropriate PD classes along these lines? Maybe we can figure out the class structure right now.

{quote}
Further more appearance generation can't be made dependent on setting the field value and only done there as there are forms where the appearance needs to be regenerated using the current field value. This is the case if the NeedAppearances flag has been set in the form. E.g. if a form has that flag set and we do render the form, prior to doing the rendering the appearance stream for the AcroForm fields needs to be (re-) generated. That is the reason why the AppearanceGenerator is publicly available and not package private and not hidden behind the field value setting.
{quote}

This is one of those low-level details which we need to figure out early on, because everything else is going to depend upon it. I agree that we need appearance generation to occur in situations other than when the field value is set, such as when rendering a field which has NeedAppearances. However, there's no reason why that code can't have PDField as its public-facing API and be package-private in pdmodel.interactive.form, e.g. if we want an appearance for a field when rendering, then we should add a getAppearance() method to PDField, which returns a generated content stream. By making the helper class public, we expose an internal implementation detail of the pdmodel.interactive.form package, which increases cross-package coupling (because PDForm still depends on that helper class) and means that those details become part of our public API and hence difficult to change - Sonar has spotted this problem in your Dec 22 commits, I've attached a screenshot in [^sonar.png].

{quote}
So .services.forms should be the new home for a higher level forms API (of course the package name could be discussed upon but I didn't want to make a package directly under o.a.pdfbox as a similar approach needs to be taken for annotations). The reason I'm establishing it now is to be able to build on that developing that higher level API without the need to do another major release to repackage. So user can expect and trust that a high level AcroForms API is in that package with further abstractions to our low level PD (or even COS) model.
{quote}

I think I've covered this in my last few comments already, so I'll just repeat that PD is our high-level API (at least for single-document manipulation) and there's nothing specific to forms which requires a higher-level API above COS and PD. But we do need to increase PD's high-level capability with regards to forms, by enhancing the PD model as I hint at above.

{quote}
Finally wrt to naming. The name PDAppearanceString has been left intact while I'm reworking the very procedural code. There is nothing like an AppearanceString in PDF. The current functionality is something between handling a Default Appearance String and creating an appearance stream.
{quote}

This is another low-level detail which we need to give some thought, as the class originally came from the PD model, it's worth asking ourselves what it really represents, and whether or not it should be replaced with something which better models how PDF actually works. At a glance it looks like this class is, as you say not really a first-class PD concept, however what it does look like is simply a helper class for PDField, which is why I say it is an implementation detail.


was (Author: jahewson):
Thanks for the detailed explanation. The changes which you're proposing to the forms API are pretty big and the specifics need to be proposed in more detail before we have something which everyone is happy with.

{quote}
The next step - and this is one of the reasons why the .services.forms package has been created - is to further enhance the usability of forms filling and creation. As of today creating a new form field users need to know that they have to create a field, have to create an annotation, set the annotation to be used within the field, apply styling to the annotation .... - not very user friendly.
{quote}

That's a very good goal but the place to add this functionality is in the PD model, as that where our high-level code lives., e.g. the new font embedding features are very sophisticated and are entirely implemented within the PD model. Convenience methods for adding form fields can be added to PDPage, perhaps by replacing List<PDAnnotation> returned by getAnnotations() with a PDAnnotations (note the 's') object which implements List<PDAnnotation> but also provides all the necessary helper methods for creating new annotations, including form fields. Alternatively there may be other designs such as having a PDFormFields getFormFields() method on PDDocument, where PDFormFields performs the high-level convince functions - whatever is the most appropriate design. Can you suggest some appropriate PD classes along these lines? Maybe we can figure out the class structure right now.

{quote}
Further more appearance generation can't be made dependent on setting the field value and only done there as there are forms where the appearance needs to be regenerated using the current field value. This is the case if the NeedAppearances flag has been set in the form. E.g. if a form has that flag set and we do render the form, prior to doing the rendering the appearance stream for the AcroForm fields needs to be (re-) generated. That is the reason why the AppearanceGenerator is publicly available and not package private and not hidden behind the field value setting.
{quote}

This is one of those low-level details which we need to figure out early on, because everything else is going to depend upon it. I agree that we need appearance generation to occur in situations other than when the field value is set, such as when rendering a field which has NeedAppearances. However, there's no reason why that code can't have PDField as its public-facing API and be package-private in pdmodel.interactive.form, e.g. if we want an appearance for a field when rendering, then we should add a getAppearance() method to PDField, which returns a generated content stream. By making the helper class public, we expose an internal implementation detail of the pdmodel.interactive.form package, which increases cross-package coupling (because PDForm still depends on that helper class) and means that those details become part of our public API and hence difficult to change - Sonar has spotted this problem in your Dec 22 commits, I've attached a screenshot in sonar.png.

{quote}
So .services.forms should be the new home for a higher level forms API (of course the package name could be discussed upon but I didn't want to make a package directly under o.a.pdfbox as a similar approach needs to be taken for annotations). The reason I'm establishing it now is to be able to build on that developing that higher level API without the need to do another major release to repackage. So user can expect and trust that a high level AcroForms API is in that package with further abstractions to our low level PD (or even COS) model.
{quote}

I think I've covered this in my last few comments already, so I'll just repeat that PD is our high-level API (at least for single-document manipulation) and there's nothing specific to forms which requires a higher-level API above COS and PD. But we do need to increase PD's high-level capability with regards to forms, by enhancing the PD model as I hint at above.

{quote}
Finally wrt to naming. The name PDAppearanceString has been left intact while I'm reworking the very procedural code. There is nothing like an AppearanceString in PDF. The current functionality is something between handling a Default Appearance String and creating an appearance stream.
{quote}

This is another low-level detail which we need to give some thought, as the class originally came from the PD model, it's worth asking ourselves what it really represents, and whether or not it should be replaced with something which better models how PDF actually works. At a glance it looks like this class is, as you say not really a first-class PD concept, however what it does look like is simply a helper class for PDField, which is why I say it is an implementation detail.

> Decouple implementation specific forms handling from interactive.form PD Model
> ------------------------------------------------------------------------------
>
>                 Key: PDFBOX-2580
>                 URL: https://issues.apache.org/jira/browse/PDFBOX-2580
>             Project: PDFBox
>          Issue Type: Improvement
>          Components: AcroForm
>            Reporter: Maruan Sahyoun
>            Assignee: Maruan Sahyoun
>             Fix For: 2.0.0
>
>         Attachments: sonar.png
>
>
> The interactive.form PD model currently holds classes reflecting the various fields intermixed with appearance generation and layout handling.
> In order to separate the PD model from the service of forms filling and appearance generation this functionality shall be moved into a new package.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org