Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation for BELL SPI visibility and BELL properties #5724

Closed
dazavala opened this issue Sep 7, 2022 · 15 comments
Closed

Documentation for BELL SPI visibility and BELL properties #5724

dazavala opened this issue Sep 7, 2022 · 15 comments
Assignees
Labels
22.0.0.11 peer reviewed technical reviewed An SME reviewed and approved the documentation from a technical perspective.
Milestone

Comments

@dazavala
Copy link

dazavala commented Sep 7, 2022

For epic OpenLiberty/open-liberty#19368.

Add sections to the Basic Extensions using Liberty Libraries 1.0 feature documentation that describe the function and usage of the new spiVisibility and <properties/> attributes in the BELL configuration.

We can use this opportunity to describe how to declare OSGi properties specific to BELL services, a necessary but yet undocumented capability, and introduce the use of the service attribute in one of the examples.

@dazavala
Copy link
Author

dazavala commented Sep 7, 2022

Hello @dmuelle. Per our previous conversation, let's schedule a few minutes to consider how to organize this content before submitting a draft. I think we need a new example section to describe extending SPI using a BELL service, and perhaps some conceptual content under section "Enabling this feature", before the examples start.

@dmuelle dmuelle self-assigned this Sep 7, 2022
@dmuelle dmuelle modified the milestones: 22.0.0.10, 22.0.0.11 Sep 7, 2022
@chirp1
Copy link
Contributor

chirp1 commented Sep 28, 2022

@dazavala @dmuelle Hi! Have you decided on what info needs to be documented? If so, would you add the info to this issue? I'll approve the epic afterwards. Thanks!

@dazavala
Copy link
Author

@dmuelle Here's my first draft: epic.19368.openliberty.io.feature.doc.bells-1.0.sep28.docx. I spent time this evening adding meta-info and transposing to MS Word.

@chirp1 Regarding what needs to be documented: The draft provides updates to the Basic Extensions using Liberty Libraries 1.0 feature documentation. The changes include conceptual example sections for the new spiVisibility and properties attributes in the BELL configuration documentation along with a new practical usage example. Changes to BELL configuration documentation are automated an must be verified at/after GA.

@dmuelle
Copy link
Member

dmuelle commented Sep 29, 2022

per slack discussion and David's comments in the draft, we'll remove the custom user registry example form the doc until we can revise and test it, which is tracked in #5850

dmuelle added a commit that referenced this issue Sep 30, 2022
@dmuelle dmuelle mentioned this issue Sep 30, 2022
dmuelle added a commit that referenced this issue Oct 3, 2022
@dmuelle dmuelle mentioned this issue Oct 3, 2022
@dmuelle
Copy link
Member

dmuelle commented Oct 3, 2022

Hi @dazavala - I've created a draft based on your word doc:

https://docs-draft-openlibertyio.mqj6zf7jocq.us-south.codeengine.appdomain.cloud/docs/latest/reference/feature/bells-1.0.html

I have a few questions to sort out in some of the examples before we do a proper review.

Configure a BELL

Be sure to enable the Open Liberty features that export the API or SPI packages that are required by the BELL service implementation classes.

This seems a little vague- can we give common 1-2 sentence example to clarify? eg
For example, if you need to use x, make sure to enable the y feature.

Specify configuration properties in BELL services

To enable property injection, define a public constructor or a public method that is named updateBell with a single parameter of the java.util.Map<String,String> type within the service implementation class.

we're asking them to "... define a public constructor or a public method that is named updateBell" but the example doesn't contain anything called updateBell- where would the method or constructor be named? Is that out of the scope of this example? Also, where would you define this method? what file?

Expose a REST endpoint for custom resources

I revised this slightly to try and draw clear connections between the examples and the descriptions- hopefully I haven't messed anything up, but it's certainly possible that I did.

I wonder if there is a better way to organize the examples in this section- it feels like it jumps around a bit (though that might just be because I'm used to documenting server.xml rather than app code). Right now we have 4 examples, in the following order:

  1. minimal service implementation class (app code, any particular file?)
  2. OSGi property (set in the META-INF/services/com.ibm.wsspi.rest.handler.RESTHandler w/in the library?)
  3. detail server config- BELL feature
  4. full server.xml example

Does this order represent how a developer would likely configure something like this? App code first, then OSGi prop, and finally the server? Just want to make it as intuitive as possible to go from one example to the next. It might help to put a 1-2 sentence overview at the beginning of the section to help the reader map the examples to the overall process.

Open Liberty extenders use the REST Handler framework SPI to expose REST endpoints (REST APIs) for custom resources.

What are Open Liberty extenders? I dont think we've used this term in OL doc before

dmuelle added a commit that referenced this issue Oct 3, 2022
@dazavala
Copy link
Author

dazavala commented Oct 4, 2022 via email

dmuelle added a commit that referenced this issue Oct 4, 2022
@dmuelle
Copy link
Member

dmuelle commented Oct 4, 2022

Thanks- I've updated the draft per your answers. When you have a chance, would you review and let me know what changes are needed?

https://docs-draft-openlibertyio.mqj6zf7jocq.us-south.codeengine.appdomain.cloud/docs/latest/reference/feature/bells-1.0.html

When you're satisfied with the draft, you can add the technical reviewed label to this issue and the doc will publish with 22.0.0.11. Thanks

@dazavala
Copy link
Author

dazavala commented Oct 18, 2022

After reviewing I have some suggestions:


Configure a BELL

  • Remove unnecessary verbiage
    For any BELL configuration, the BELL feature discovers all services in the META-INF/services folder of the referenced library.
    --> The BELL feature discovers all services in the META-INF/services folder of the referenced library.

  • Maintain congruence with bell config doc
    Alternatively, you can configure the service attribute to specify a service for Open liberty to look up in the META-INF/service folder, as shown in the following example.
    --> Alternatively, you can configure the service attribute to specify the name of the service the feature will look up in the META-INF/service folder, as shown in the following example.

  • Fix grammar regarding singularity
    For all services that are discovered or specified, ...
    --> For every service that is discovered or specified, ...

  • Remove this. Seems unnecessary on second glance
    For example, you must enable the Jakarta Servlet feature for a library that provides an implementation of the jakarta.servlet.ServletContainerInitializer interface because this feature provides the jakarta.servlet API package.


Implement an SPI with BELL services

  • Change section title to be precise, use term "access" as user docs do for API type visibility.
    Implement an SPI with BELL services
    --> Enable SPI access for libraries

  • User term "access" for SPI; remove reference to API package visibility, which seems unnecessary; describe the effected library classloader behavior.
    Shared libraries do not support visibility for SPI packages like they do for API packages. When you deploy BELL services
    that implement interfaces in SPI packages, you must set the spiVisibility attribute to true in the BELL configuration. This attribute enables the referenced library to see SPI packages.
    --> Shared libraries do not support access for SPI packages. When a library provides an implementation
    of an SPI interface, set the spiVisibility attribute to true in the BELL configuration. The attribute enables the library classloader to access SPI packages provided by Open Liberty.

  • Shortened name of REST Endpoint section
    For more information, see Expose a REST endpoint for custom resources
    --> For more information, see Expose a new REST endpoint.


Specify configuration properties in BELL services

  • Change title based on similar doc for user feature support.
    Specify configuration properties in BELL services
    --> Enable services to receive configuration properties

  • Modify first sentence to agree with new title; introduce notion of injection enablement before describing how to in next paragraph.
    Service implementation classes can receive properties that are declared in the BELL configuration. To configure properties, add a properties element to your BELL configuration and declare one or more name="value" attributes. Properties are String type and apply to any services that are provided by the referenced library that are enabled for property injection. The following example declares the hello and serverHome properties.
    --> Services can receive properties that are declared in the BELL configuration. To configure properties, add a properties element to your BELL configuration and declare one or more name="value" attributes. Properties are String type and will inject into all services that are enabled (coded) to receive them. The following example declares the hello and serverHome properties.

  • Fix a conceptual mixup regarding parameter description (open to further improvement); be consistent with previous paragraph regarding injection enablement.
    To enable property injection for a service, define either a public method that is named updateBell or a public constructor.
    Set a single parameter of the java.util.Map<String,String> type within the service implementation class. The following example shows a single-argument constructor to receive properties from the BELL configuration.
    -->To enable (code) a service to receive configuration properties, define a public method named updateBell or a public constructor in the service implementation class. The method signature must declare a single parameter of the java.util.Map<String,String> type. The following example defines both methods to receive properties in a BELL configuration.

  • Notice changes to the class: Removed props field and related code in method definitions. Only method signatures matter - no need to define method behaviors. Users can do that, and later we provide a simple functional example.

public class YourServiceImpl implements api.or.spi.Interface {
   public YourServiceImpl(java.util.Map<String,String> bellProperties) {...}
   // OR
   public void updateBell(java.util.Map<String,String> bellProperties) {...}
 }

Expose a REST endpoint for custom resources

  • Change title. No need to introduce notion of custom resources. The intent is implied by the concept of a custom REST endpoint.
    Expose a REST endpoint for custom resources
    --> Expose a new REST endpoint

  • Replace the opening paragraph, below. Mimmick the "liberty extender" description introduced here to remove redundant "you can use" phrases and to afford description of bell usage in terms of library configuration like the other examples.
    _ _
    When you develop an extension to the Open Liberty runtime, you can use the REST Handler framework SPI to expose REST endpoints (REST APIs) for custom resources. You can use the BELL feature to expose a REST endpoint by providing a service that implements the com.ibm.wsspi.rest.handler.RESTHandler REST Handler SPI interface.
    -->The REST Handler framework provides the com.ibm.wsspi.rest.handler.RESTHandler interface which Open Liberty extenders use to expose new REST endpoints. You can use the BELL feature to configure a library that provides a RESTHandler implementation as a service and enables the REST Handler framework to register the service as a listener for a new, specified endpoint URL.

  • Reduce the following paragraph. Library usage is now described above. Note that the BellEndpoint class is not the server resource -- that's the file in the /META-INF/services directory.
    _ _
    In the following service implementation class example, a library provides an implementation of the RestHandler interface. The library also provides a service resource that is called BellEndpoint. This resource enables the REST Handler framework to register the service as a listener at a specified subroot of the /ibm/api URL path. This minimal service implementation class also defines a single-argument constructor to receive BELL configuration properties at service creation.
    --> In the following service implementation example, the BellEndpoint class overrides the handleRequest method and defines a single-argument constructor to receive BELL configuration properties at service creation.

  • We can reduce the following paragraph. A later paragraph indicates where the library jar is copied/deployed. Also, no need to bullet the file names. The intent is a listing of jar contents.
    _ _
    For this example, the following service resource and implementation class files are packaged in the RestEpLib.jar file, which is available to the Open Liberty server configuration.
    --> For this example, the following service resource and implementation class files are packaged in the RestEpLib.jar file:

META-INF/services/com.ibm.wsspi.rest.handler.RESTHandler
your/org/rest/example/BellEndpoint.class
  • _Subtle. The current paragraph is technically correct. Looking to emphasize the inclusion of the OSGi property as a special case. The service class name is always required in the service resource. _
    _ _
    To enable the endpoint, the META-INF/services/com.ibm.wsspi.rest.handler.RESTHandler service resource file must contain the name of the service implementation class and the com.ibm.wsspi.rest.handler.root OSGi service property.
    -->To enable the endpoint, the /META-INF/services/com.ibm.wsspi.rest.handler.RESTHandler service resource must declare the com.ibm.wsspi.rest.handler.root OSGi service property in addition to the fully-qualified name of the service implementation class.

  • Is there a way to depict this paragraph as a technical aside?
    You can configure OSGi properties that are specific to a service implementation class by adding one or more properties, prefixed by the # character, to the service resource. Add the properties immediately before the name of the service implementation class to which they apply. The BELL feature registers the service interface with the implementation class and the specific OSGi properties.

  • Wasn't keen on the phrase "to support the endpoint". Notice the term "see" changed to "access". What do you think?
    _ _
    The following example shows the BELL configuration in the server.xml to support the endpoint. The BELL configuration specifies the spiVisibility="true" attribute to enable the RestEpLib library to see the REST Handler SPI packages. The RestEpLib.jar library JAR is deployed at to the ${server.config.dir}/sharedlib directory and the configuration declares one BELL property that is named hello.
    -->The following example shows the BELL configuration in server.xml that references the RestEpLib library. The configuration specifies the spiVisibility="true" attribute to enable the RestEpLib library to access the REST Handler SPI packages. It also declares one BELL property. Notice the RestEpLib.jar library JAR is copied to the ${server.config.dir}/sharedlib directory.

  • _Be consistent with feature docs, which use the term "provides" rather than "exports". Sorry about that. _
    _ _
    The Admin REST Connector feature exports the REST Handler framework SPI.
    --> The Admin REST Connector feature provides the REST Handler framework SPI.

dmuelle added a commit that referenced this issue Oct 18, 2022
@dazavala
Copy link
Author

@dmuelle I've posted all change suggestions. Seems premature to add the technical reviewed label at this time. I'll be available tomorrow, all day AFAIK, to assist with any dubious suggestions and to complete the process points for this issue.

dmuelle added a commit that referenced this issue Oct 18, 2022
@dmuelle dmuelle mentioned this issue Oct 18, 2022
dmuelle added a commit that referenced this issue Oct 19, 2022
@dmuelle dmuelle mentioned this issue Oct 19, 2022
@dmuelle
Copy link
Member

dmuelle commented Oct 19, 2022

@dazavala - all suggestions implemented except-

Is there a way to depict this paragraph as a technical aside?

There is not a great way to do this in the OL Docs. The "Note" formatting in Antora runs afoul of our accessibility scans. I tired simply adding "Note:" but it seemed more distracting that anything else.

New draft: https://docs-draft-openlibertyio.mqj6zf7jocq.us-south.codeengine.appdomain.cloud/docs/latest/reference/feature/bells-1.0.html

@dazavala
Copy link
Author

dazavala commented Oct 19, 2022

The new draft looks really great. I found few items. Items 2, 3, 5 are mandatory.

  • I'm unsure whether the term "String" in the phrase "Properties are String type and ..." needs code formatting. It's the name of a type in the property configuration, not a Java type name (like java.lang.String). Ultimately, the string value in the property configuration becomes a java.lang.String at runtime. The same would apply e.g. to the spiVsibility attribute, which has type Boolean but is not a Java type name. You probably have it right.

  • Remove duplicate word "named" from "define a public method that is named named updateBell or ...". I would sure like to remove "that is named" and simple say "... public method updateBell or ...".

  • Change parameter Map<String,String> bellProps to java.util.Map<String,String> bellProperties in the methods YourServiceImpl() and updateBell() of example class YourServiceImpl. We removed the import statement from the example, so we have to use the fully-qualified package name (java.util.Map) in the method signatures. I want the longer term bellProperties to further differentiate them from OSGI service properties.

  • Can we change this phrase "When this server starts, the BELL feature uses the RestEpLib library to register the..." to "When this server starts, the BELL feature registers the ..." ?

  • Change this phrase "At service creation, the BELL feature invokes the updateBell constructor or methods to..." to "At service creation, the BELL feature invokes the updateBell or constructor methods to"

dmuelle added a commit that referenced this issue Oct 19, 2022
@dmuelle dmuelle mentioned this issue Oct 19, 2022
@dazavala
Copy link
Author

@dmuelle Rock on!

@dazavala dazavala added the technical reviewed An SME reviewed and approved the documentation from a technical perspective. label Oct 19, 2022
@ramkumar-k-9286
Copy link
Contributor

Peer Review - Basic Extensions using Liberty Libraries

Short Description - Just one sentence. Rephrase by adding 2nd sentence to the short desc.


With the BELL feature, a library declares a /META-INF/services/ resource with the name of the fully qualified API or SPI interface, and provides a class that implements the interface.
-->
Acrolinx picking up the ,
With the BELL feature, a library declares a /META-INF/services/ resource with the name of the fully qualified API or SPI interface and provides a class that implements the interface.


The following example shows the BELL configuration in server.xml file that references the RestEpLib library.
-->
The following example shows the BELL configuration in the server.xml file that references the RestEpLib library.


The Transport Security feature and the related keyStore, basicRegistry, and adminstrator-role configurations support secure access to the endpoint.
-->
spell check -> administrator-role


The BELL feature does not support nondestructive updates.
-->
The BELL feature does not support non-destructive updates.


dmuelle added a commit that referenced this issue Oct 20, 2022
@dmuelle
Copy link
Member

dmuelle commented Oct 20, 2022

Thanks for reviewing @ramkumar-k-9286 , all suggestions implemented except

Short Description - Just one sentence. Rephrase by adding 2nd sentence to the short desc.

This is in the autogenerated portion of the page, not in our files. Generally, we don't strictly enforce the 2 sentence rule on feature pages since that content is in the metatype

The BELL feature does not support non-destructive updates.

Acrolinx says "Don't use a hyphen after the prefix non"

@dmuelle
Copy link
Member

dmuelle commented Oct 21, 2022

updates are on vNext and will publish with 22.0.0.11- closing this issue.

@dmuelle dmuelle closed this as completed Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
22.0.0.11 peer reviewed technical reviewed An SME reviewed and approved the documentation from a technical perspective.
Projects
None yet
Development

No branches or pull requests

4 participants