Added some logic into JeevesNodeAwareLogoutSuccessHandler to deal with the case where no port is set in the settings manager

[archaeogeek]

This PR handles the error on signout where there is no port set in the settings manager. Previously if no port was set (because it's not needed) logging out would produce a null pointer error (see below):

This PR adds some logic to check for the value of the port in the settings and to set it to 80 if it is not included.
With this pull request, if no port is set the logout error no longer occurs.

[archaeogeek]

@josegar74 thanks for the suggested changes, I have made them and re-tested

[josegar74]

@archaeogeek the code works fine, but I'm checking additional code and setting the port to empty it can cause issues in other code like this one, if the value is empty it will add the : character:

core-geonetwork/core/src/main/java/org/fao/geonet/kernel/datamanager/base/BaseMetadataManager.java

Lines 920 to 925 in 376e9e6

	String port = settingManager.getValue(Settings.SYSTEM_SERVER_PORT);
	if (port.equals("80")) {
	port = "";
	} else {
	port = ":" + port;
	}

Note also that xslt process code can have the same problem as the previous class:

core-geonetwork/web/src/main/webapp/xslt/services/dcat/rdf.xsl

Lines 40 to 47 in bb48909

	<xsl:variable name="port"> <xsl:value-of select="$env/system/server/port"/></xsl:variable>
	<xsl:variable name="url"
	select="concat($env/system/server/protocol, '://',
	$env/system/server/host,
	if (($env/system/server/protocol = 'http' and $port = '80') or
	($env/system/server/protocol = 'https' and $port = '443')) then '' else concat(':', $port),
	/root/gui/url)"/>

I would do the following:

1. Change https://github.com/geonetwork/core-geonetwork/blob/main/core/src/main/java/org/fao/geonet/kernel/setting/SettingInfo.java to add this method:

    public int getSitePort() {
        SettingManager settingManager = ApplicationContextHolder.get().getBean(SettingManager.class);

        // some conditional logic to handle the case where there's no port in the settings
        int sitePort = Geonet.DefaultHttpPort.HTTP;
        if (StringUtils.isNumeric(settingManager.getValue(Settings.SYSTEM_SERVER_PORT))) {
            sitePort = settingManager.getValueAsInt(Settings.SYSTEM_SERVER_PORT);
        }

        return sitePort;
    }

And use it in your change of JeevesNodeAwareLogoutSuccessHandler.java and in BaseMetadataManager.java

[archaeogeek]

@josegar74 I've tried to do what you suggested, but have ended up with compilation errors: Here's what I've done:

Add the following after getSiteUrl in SettingInfo.java, along with adding a line import org.apache.commons.lang3.StringUtils;:

 /**
   * Handle the case where the port in Settings is empty
  */

    public Integer getSitePort() {
             SettingManager settingManager = ApplicationContextHolder.get().getBean(SettingManager.class);

        // some conditional logic to handle the case where there's no port in the settings
        int sitePort = Geonet.DefaultHttpPort.HTTP;
        if (StringUtils.isNumeric(settingManager.getValue(Settings.SYSTEM_SERVER_PORT))) {
            sitePort = settingManager.getValueAsInt(Settings.SYSTEM_SERVER_PORT);
        }

        return sitePort;
    }

Change BaseMetadataManager.java like so (eg by commenting out the logic for picking up the port and adding the new line), and adding the line import org.fao.geonet.kernel.setting.SettingInfo;:

String protocol = settingManager.getValue(Settings.SYSTEM_SERVER_PROTOCOL);
String host = settingManager.getValue(Settings.SYSTEM_SERVER_HOST);
//String port = settingManager.getValue(Settings.SYSTEM_SERVER_PORT);
       //if (port.equals("80")) {
        //    port = "";
        //} else {
        //    port = ":" + port;
        //}
String port = Integer.toString(SettingInfo.getSitePort());

Changing JeevesNodeAwareLogoutSuccessHanlder.java by adding the line import org.fao.geonet.kernel.setting.SettingInfo; and changing the lines to get the port:

// some conditional logic to handle the case where there's no port in the settings
//int sitePort = Geonet.DefaultHttpPort.HTTP;
//if (StringUtils.isNumeric(settingManager.getValue(Settings.SYSTEM_SERVER_PORT))) {
//    sitePort = settingManager.getValueAsInt(Settings.SYSTEM_SERVER_PORT);
// }

int sitePort = SettingInfo.getSitePort(); 

When I then build with maven I get the following error:

[ERROR] /home/jo/Git/core-geonetwork/core/src/main/java/org/fao/geonet/kernel/datamanager/base/BaseMetadataManager.java:[927,51] non-static method getSitePort() cannot be referenced from a static context
[ERROR] /home/jo/Git/core-geonetwork/core/src/main/java/jeeves/config/springutil/JeevesNodeAwareLogoutSuccessHandler.java:[98,47] non-static method getSitePort() cannot be referenced from a s
tatic context

Can you see what I'm doing wrong?

[archaeogeek]

@josegar74 could you check my code to see where I am going wrong?

[archaeogeek]

@josegar74 Hopefully these latest changes fix the issues you reported- I'd be grateful if this could make it into the 4.2.5 release if possible!

[josegar74]

@archaeogeek I'm going to check the code, thanks and apologies for the late reply, missing your previous comments to review the code.

[josegar74]

@archaeogeek I've added some code suggestions, to remove commented code and fix a wrong tab in a code line. You can commit them from GitHub.

Other than that, looks working ok, thanks.

[archaeogeek]

@josegar74 Great, is it OK to merge this now?