sirthias / pegdown

A pure-Java Markdown processor based on a parboiled PEG parser supporting a number of extensions
http://pegdown.org
Apache License 2.0
1.29k stars 218 forks source link

OSGi Meta Information is not complete #62

Open robertcsakany opened 11 years ago

robertcsakany commented 11 years ago

The MANIFEST.MF contains some basic tags to be able to work with in an OSGi container, but the main problem is the Export-Package and Import-Package tag is not contained. So it is deployed to container but no other bundle can access it because nothing is exported. (I think all of org.pegdown have to exported)

robertcsakany commented 11 years ago

There is two way to resolve this. First, the pegdown builder fix (for fix for main package) The drawback of this solution is hard to maintain the import and exports

diff --git a/buildfile b/buildfile
index 9f3917a..6b55aee 100644
--- a/buildfile
+++ b/buildfile
@@ -28,7 +28,8 @@ define 'pegdown' do
   manifest['Bundle-DocURL'] = 'http://www.pegdown.org'
   manifest['Bundle-Vendor'] = 'pegdown.org'
   manifest['Bundle-SymbolicName'] = 'org.pegdown'
-  
+  manifest['Export-Package'] = 'org.pegdown.ast;uses:="org.parboiled.common,org.parboiled.trees";version="1.1.0",org.pegdown;uses:="org.parboiled.common,org.pegdown.ast,org.parboiled.parserunners,org.parboiled,org.parboiled.buffers,org.parboiled.erors,org.parboiled.support,org.parboiled.annotations";version="1.1.0"'
+  manifest['Import-Package'] ='org.parboiled;version="[1.1.3,2.0.0)",org.parboiled.annotations;version="[1.1.3,2.0.0)",org.parboiled.buffers;version="[1.1.3,2.0.0)",org.parboiled.common;version="[1.1.3,2.0.0)",org.parboiled.errors;version="[1.1.3,2.0.0)",org.parboiled.matchers;version="[1.1.3,2.0.0)",org.parboiled.matchervisitors;version="[1.1.3,2.0.0)",org.parboiled.parserunners;version="[1.1.3,2.0.0)",org.parboiled.support;version="[1.1.3,2.0.0)",org.parboiled.transform;version="[1.1.3,2.0.0)",org.parboiled.trees;version="[1.1.3,2.0.0)"'
   meta_inf << file('NOTICE')

   PARBOILED_VERSION = '1.0.2'```

The scond, create a new subproject, called pegdown-osgi, create a pom.xml file in it and run with maven:

```xml
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
  <modelVersion>4.0.0</modelVersion>
  <groupId>org.pegdown</groupId>
  <artifactId>pegdown-osgi</artifactId>
  <version>1.1.0</version>
  <name>Pegdown Markup language processor</name>
  <description>Pegdown is a pure Java library for clean and lightweight Markdown processing. It's implementation is based on a parboiled PEG parser and is therefore rather easy to understand and extend.
  </description>
  <properties>
    <parboiled.version>1.1.3</parboiled.version>
  </properties>
  <inceptionYear>2010</inceptionYear>
  <packaging>bundle</packaging>
  <licenses>
    <license>
      <name>Apache 2</name>
      <url>http://www.apache.org/licenses/LICENSE-2.0.txt</url>
      <distribution>repo</distribution>
    </license>
  </licenses>

  <build>
    <plugins>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>2.3.2</version>
        <configuration>
          <source>1.5</source>
          <target>1.5</target>
        </configuration>
      </plugin>

            <plugin>
                <groupId>org.apache.felix</groupId>
                <artifactId>maven-bundle-plugin</artifactId>
                <extensions>true</extensions>
                <configuration>
                    <instructions>

                        <Embed-Dependency>pegdown;inline=true</Embed-Dependency>
                        <Embed-Transitive>true</Embed-Transitive>

                        <_exportcontents>
                            org.pegdown.*;version=${project.version}
                        </_exportcontents>

                        <Import-Package>
                            org.parboiled*;version="[1.1.3,2.0.0)",
                            org.parboiled.matchers;version="[1.1.3,2.0.0)",
                            org.parboiled.matchervisitors;version="[1.1.3,2.0.0)",
                            org.parboiled.transform;version="[1.1.3,2.0.0)"
                        </Import-Package>
                    </instructions>
                </configuration>
            </plugin>
    </plugins>
  </build>

  <dependencies>
    <dependency>
      <groupId>org.pegdown</groupId>
      <artifactId>pegdown</artifactId>
      <version>${project.version}</version>
    </dependency>
  </dependencies>
</project>
rocketraman commented 11 years ago

I think you pasted the wrong POM above?

robertcsakany commented 11 years ago

Thnaks for your report. Yes, it was an older version and the exports was incorrect. Please check it now!

rocketraman commented 11 years ago

I've found during my testing that pegdown requires imports from parboiled, BUT parboiled also needs to import classes from pegdown. I noticed you added a TCCL in parboiled to try and work around this, and I got past the point where the TCCL was invoked, but still ran into this error:

...
Caused by: java.lang.RuntimeException: Error creating extended parser class: null
        at org.parboiled.Parboiled.createParser(Parboiled.java:58)
        at org.pegdown.PegDownProcessor.<init>(PegDownProcessor.java:66)
        at org.pegdown.PegDownProcessor.<init>(PegDownProcessor.java:57)
        at org.fusesource.scalate.filter.pegdown.PegDownFilter$.<init>(PegDownFilter.scala:31)
        at org.fusesource.scalate.filter.pegdown.PegDownFilter$.<clinit>(PegDownFilter.scala)
        ... 24 more
Caused by: java.lang.NullPointerException
        at java.lang.Class.isAssignableFrom(Native Method)
        at org.parboiled.transform.RuleMethod.visitLocalVariable(RuleMethod.java:308)
        at org.objectweb.asm.ClassReader.accept(Unknown Source)
        at org.objectweb.asm.ClassReader.accept(Unknown Source)
        at org.parboiled.transform.ClassNodeInitializer.process(ClassNodeInitializer.java:63)
        at org.parboiled.transform.ParserTransformer.extendParserClass(ParserTransformer.java:44)
        at org.parboiled.transform.ParserTransformer.transformParser(ParserTransformer.java:38)
        at org.parboiled.Parboiled.createParser(Parboiled.java:54)
        ... 28 more

You can see the parboiled is doing some stuff later that requires the pegdown class to be visisble from the parboiled classloader.

rocketraman commented 11 years ago

PS I've worked around this temporarily by adding optional imports in parboiled to pegdown:

                       <!-- shouldn't have to do this but parboiled requires access to the implementation bundles -->
                        <Import-Package><![CDATA[
                           org.pegdown;version="[1.2.0, 1.3.0)";resolution:="optional",
                           org.pegdown.ast;version="[1.2.0, 1.3.0)";resolution:="optional",
                           *
                        ]]>

Obviously that isn't an elegant solution. Ideally, for OSGi parboiled would expose and consume services...

Also, I had to add the folllowing additional imports to pegdown:

                        <Import-Package>
                          org.parboiled*;version="[1.1.3,2.0.0)",
                          org.parboiled.matchers;version="[1.1.3,2.0.0)",
                          org.parboiled.matchervisitors;version="[1.1.3,2.0.0)",
                          org.parboiled.transform;version="[1.1.3,2.0.0)"
                        </Import-Package>
robertcsakany commented 11 years ago

Yes, I know the problem. The problem is a design problem when cross references two bundle. There is more solution for it.

First, have to split the packeges to more slice avoid cross references. This is the most standard solution for cross references. But in this case is other OSGi killer problem too, the classLoading via name.

Second (I don't like it too much) the DynamicImport-Package. (It is more 'standard' than CDATA embedding of imports). The drawback is the dynamic import tries all of bundles exports, it can casuse performance issues.

The another problem is the classLoading by name.(via String). This causes the bundle-plugin cannot create bundle imports correctly.

My solution was different. In my services (that uses pegdown) there is a dynamicClassLoader that uses PackageAdmin to get the bundle's exports and making a composite classloader of the given bundles (in our case parboiled-osgi and pegdown-osgi and the service bundle itself). After in my service I'm setting it to contextClassLoader and init the pegdown with it. I know it is not standard OSGi way (Yes, I had to code a listener to check the Bundle's state chages to refresh the states of classLoaders), but in other monolotic stuff (like Solr, Gwt) I'm using this solution also. To achieve this you need my another patch in parboiled (#51) . If you have better solution I will appreciate if you share me.

robertcsakany commented 11 years ago

I've made the fixes you suggested. Than you very much for them! (Import-Packages) changes and the Second solution on Parboiled, and it works flawlassly without my classLoader hack (In my solution i'm uing mine, but in this "official" case is absolutely irrevelant). The DynamicImport-Package OSGi compliance solution, but it has its own drawbacks. There is a good article about this problem http://billhiggins.us/blog/2011/02/24/using-reflection-in-osgi/ .

rocketraman commented 11 years ago

I like to avoid DynamicImport-Package whenever possible -- it really breaks the encapsulation OSGi is supposed to provide, and encourages code that doesn't respect module boundaries. The "right" solution in this case, as I mentioned, might be an OSGi service layer on top of parboiled, which might require a few changes (but probably relatively minor) in the parboiled core. This type of solution could be made to work within and without an OSGi environment. Neil Bartlett has a few hints in this direction in the comments of the blog you linked to.

elennaro commented 9 years ago

I'm stuck facing same problems (pegdown v1.4.2, JDK 8) while trying to implement custom ParserPlugin to a library I'm writing: CustomPlugin:

public class CustomHeadersParserPlugin extends Parser implements BlockPluginParser {

    public CustomHeadersParserPlugin() {
        super(HtmlMdProc.MDP_SETTINGS, HtmlMdProc.PROCESSING_TIME_LIMIT, DefaultParseRunnerProvider);
    }

    public CustomHeadersParserPlugin(Integer options, Long maxParsingTimeInMillis) {
        super(options, maxParsingTimeInMillis, DefaultParseRunnerProvider);
    }

    public CustomHeadersParserPlugin(Integer options, Long maxParsingTimeInMillis, ParseRunnerProvider parseRunnerProvider) {
        super(options, maxParsingTimeInMillis, parseRunnerProvider);
    }

    public CustomHeadersParserPlugin(Integer options, Long maxParsingTimeInMillis, ParseRunnerProvider parseRunnerProvider,
        PegDownPlugins plugins) {
        super(options, maxParsingTimeInMillis, parseRunnerProvider, plugins);
    }

    //************* CUSTOM RULES ****************

Pegdown Usage:

public class HtmlMdProc {

public static final int MDP_SETTINGS
        = Extensions.HARDWRAPS | Extensions.AUTOLINKS | Extensions.TABLES | Extensions.FENCED_CODE_BLOCKS;
    public static final long PROCESSING_TIME_LIMIT = 5000l;
...
public HtmlMdProc markdown() {
        PegDownPlugins pdp = PegDownPlugins.builder()
            .withPlugin(CustomHeadersParserPlugin.class).build();
        PegDownProcessor mdp = new PegDownProcessor(MDP_SETTINGS, PROCESSING_TIME_LIMIT, pdp);
        RootNode rn = mdp.parseMarkdown(text.toCharArray());
        String result = new CustomMarkdownToHtmlSerializer().toHtml(rn);
        if (result != null)
            this.text = result;
        else
            logger.debug("Could not process markdown in {} seconds", PROCESSING_TIME_LIMIT / 1000);
        return this;
    }

Test:

    @Test
    public void testmarkdownWithoutCode() {
        String before = "Simple new line\nTest\n\nTest\nVot";
        String expected = "<p>Simple new line<br />Test</p><p>Test<br />Vot</p>".replaceAll("\r", "");
        HtmlMdProc textProc = new HtmlMdProc(before);
        String result = textProc.markdown().text();
        assertEquals(expected, result);
    }

Library is a Maven project. Testing Exeption:

java.lang.RuntimeException: Error creating extended parser class: null
    at org.objectweb.asm.ClassReader.<init>(Unknown Source)
    at org.objectweb.asm.ClassReader.<init>(Unknown Source)
    at org.objectweb.asm.ClassReader.<init>(Unknown Source)
    at org.parboiled.transform.AsmUtils.createClassReader(AsmUtils.java:56)
    at org.parboiled.transform.ClassNodeInitializer.process(ClassNodeInitializer.java:62)
    at org.parboiled.transform.ParserTransformer.extendParserClass(ParserTransformer.java:44)
    at org.parboiled.transform.ParserTransformer.transformParser(ParserTransformer.java:38)
    at org.parboiled.Parboiled.createParser(Parboiled.java:54)
    at org.pegdown.plugins.PegDownPlugins$Builder.withPlugin(PegDownPlugins.java:113)
    at com.myorg.html.services.HtmlMdProc.markdown(HtmlMdProc.java:317)
    at com.myorg.html.services.HtmlMdProcTest.testmarkdownWithoutCode(HtmlMdProcTest.java:262)

Please, experts help me!

  1. Can I somehow bind my CustomHeadersParserPlugin avoiding spooky Reflections?
  2. If not, please, please, tell me how to setup maven-bundle-plugin in pom.xml to make it work with pegdown v 1.4.2, I'm a very novice so need your help very much!