Some links between markdown files point to non-existing pages

[cmanique]

Hello,

Running version 7.12 of plugin with Java 8 or 17, against confluence server 7.x.

Please find below the steps to try and reproduce links being generated to non-existing pages.

site.yaml

home:
  uri: home.md
  children:
    - name: Page-A
      uri: a.md
      children:
        - name: Page-B
          uri: b.md
        - name: Page-C
          uri: c.md

a.md

# Page A
- [This is Page-B Link](b.md)
- [This is Page-C Link](c.md)

b.md

# Page B
- [This is Page-A Link](a.md)
- [This is Page-C Link](c.md)

c.md

# Page C
- [This is Page-A Link](a.md)
- [This is Page-B Link](b.md)

The page structure is correctly published to confluence:

However some of the links are not correctly generated, as you can see on the storage format for each of the pages:

Page-A storage format:

<h1>Page A</h1>
<ul>
  <li>
    <ac:link>
      <ri:page ri:content-title="TCAAS3218-466 - Page-B"/>
      <ac:link-body>This is Page-B Link</ac:link-body>
    </ac:link>
  </li>
  <li>
    <ac:link>
      <ri:page ri:content-title="TCAAS3218-466 - Page-C"/>
      <ac:link-body>This is Page-C Link</ac:link-body>
    </ac:link>
  </li>
</ul>

Page-B storage format:

<h1>Page B</h1>
<ul>
  <li>
    <ac:link>
      <ri:page ri:content-title="TCAAS3218-466 - TCAAS3218-466 - Page-A"/>
      <ac:link-body>This is Page-A Link</ac:link-body>
    </ac:link>
  </li>
  <li>
    <ac:link>
      <ri:page ri:content-title="TCAAS3218-466 - Page-C"/>
      <ac:link-body>This is Page-C Link</ac:link-body>
    </ac:link>
  </li>
</ul>

Page-C storage format:

<h1>Page C</h1>
<ul>
  <li>
    <ac:link>
      <ri:page ri:content-title="TCAAS3218-466 - TCAAS3218-466 - Page-A"/>
      <ac:link-body>This is Page-A Link</ac:link-body>
    </ac:link>
  </li>
  <li>
    <ac:link>
      <ri:page ri:content-title="TCAAS3218-466 - TCAAS3218-466 - Page-B"/>
      <ac:link-body>This is Page-B Link</ac:link-body>
    </ac:link>
  </li>
</ul>

It seems like the pages are being processed/created in ascending order, and the only links that are broken are the ones for pages that didn’t exist at the time (they have an extra TCAAS3218-466).
TCAAS3218-466 in this case is the value of the pageTitle parameter (didn’t try this with the default that uses maven cords, but I’d naifly argue it’s not related).

By glancing at the repo I'm tempted to say the code below is the likely culprit:

maven-confluence-plugin/processor-commonmark/src/main/java/org/bsc/markdown/commonmark/CommonmarkConfluenceWikiVisitor.java

Lines 279 to 290 in e3810ee

	public void visit(Link node) {
	final String destination = processLinkUrl(node.getDestination(), parseContext);
	processChildren(node)
	//.preAndPost("<<LNK>>")
	.pre(() -> "[" )
	.captureOutput( v -> buffer().append(v) ) // ignore text
	.post(() -> format("|%s%s]", destination, ofNullable(node.getTitle()).map( v -> "|"+v ).orElse("")) )
	.process()
	.nl(isParentRoot(node));
	}

Maybe it needs a second pass to recheck destinations that were not yet present, or something of the sort.

[bsorrentino]

Hi @cmanique

I've read the related (closed) issue CONFCLOUD-69902. The proposed solution is not easy to apply in such plugin because currently it generates the storage format using a confluence builtin conversion:

Conversion pipeline

MARKDOWN -[plugin]-> WIKI -[confluence]-> STORAGE FORMAT

As you can see it is not possible apply the proposed solution because plugin control the generation pipeline until WIKI generation. I'll investigate if is possible make something from wiki format itself.

Workaround

At the moment, the only workaround I see is to not use format Markdown/Wiki but to write the page directly in storage format, feature supported by this plugin

Let me know your thoughts

[cmanique]

Hi @bsorrentino thanks for your quick reply.

I really think this is a bug originating from

maven-confluence-plugin/core/src/main/java/org/bsc/markdown/MarkdownVisitorHelper.java

Lines 113 to 119 in e3810ee

	return parseContext.getSite()
	.flatMap( site -> site.getHome().findPage( comparePath ) )
	.map( page -> parseContext.getPagePrefixToApply()
	.filter( prefixToApply -> !url.startsWith(prefixToApply) )
	.map( prefixToApply -> format( "%s - %s", prefixToApply, page.getName() ) )
	.orElse( page.getName() ) )
	.orElse(url)

which is called from

maven-confluence-plugin/processor-commonmark/src/main/java/org/bsc/markdown/commonmark/CommonmarkConfluenceWikiVisitor.java

Line 281 in e3810ee

final String destination = processLinkUrl(node.getDestination(), parseContext);

There are no checks if the prefix is being applied more than once. Hence the wrong links once a Site.Page was already processed.

I cloned the repo locally and did some tests with this quirky (changing inside the lambda would be a big departure from the current implementation) fix, that seems to mitigate the problem:

        String linkUrl = parseContext.getSite()
                .flatMap( site -> site.getHome().findPage( comparePath ) )
                .map( page -> parseContext.getPagePrefixToApply()
                        .filter( prefixToApply -> !url.startsWith(prefixToApply) )
                        .map( prefixToApply -> format( "%s - %s", prefixToApply, page.getName() ) )
                        .orElse( page.getName() ) )
                .orElse(url);
        String pagePrefixToApply = parseContext.getPagePrefixToApply().get();
        if(linkUrl.startsWith(format("%s - %s - ",pagePrefixToApply,pagePrefixToApply))){
            linkUrl = parseContext.getSite().flatMap( site -> site.getHome().findPage( comparePath)).map( p -> p.getName()).orElse(linkUrl);
        }
        return linkUrl;

I'm sure it needs real testing to ensure it doesn't break anything else, but you think a bug fix with this direction would be possible?

Thanks for the great support and all the work with the tool

[bsorrentino]

Hi @cmanique

There is no need to change stream pipeline I've applied your suggestion and tested it with success without interrupt flow.

return parseContext.getSite()
              .flatMap( site -> site.getHome().findPage( comparePath ) )
              .map( page -> parseContext.getPagePrefixToApply()
                      .map( prefixToApply -> prefixToApply.concat(" - "))
                      .filter( prefixToApply -> !url.startsWith(prefixToApply) ) // check prefix already applied
                      .filter( prefixToApply -> !page.getName().startsWith(prefixToApply) ) // check prefix already applied
                      .map( prefixToApply -> prefixToApply.concat( page.getName() ) )
                      .orElse( page.getName() ) )
              .orElse(url)
              ;

I've deployed developer release 7.13-SNAPSHOT, please could you test it and let me know ?

[cmanique]

Hi @bsorrentino,

Your code works if I build it locally, but with 7.13-SNAPSHOT from https://oss.sonatype.org/content/repositories/snapshots keeps the same behavior. The decompiled class from the snapshot jar doesn't seem to have those changes.

[bsorrentino]

it's weird🤔. I've redeployed

[cmanique]

Hi @bsorrentino , just ran with -U and got a new jar.
It is working fine now.
Any idea when 7.13 will be released?
Thanks a lot for the super fast resolution!

[bsorrentino]

Hi @bsorrentino , just ran with -U and got a new jar. It is working fine now. Any idea when 7.13 will be released? Thanks a lot for the super fast resolution!

I've planned to fix also #295 after that I'll release official 7.13 probably at end of this week

[bsorrentino]

Hi @cmanique release 7.13 is out!

[cmanique]

Thanks again for the awesome work and fast support @bsorrentino