Skip to content

refactor: Modernize ChaincodeBase class #357

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

refactor: Modernize ChaincodeBase class #357

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54228ee
refactor: Modernize ChaincodeBase class
bhaskarvilles Oct 21, 2024
1917b78
Refactor ChaincodeBase for Java 8 compatibility and code simplification
bhaskarvilles Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 52 additions & 92 deletions fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/shim/ChaincodeBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,24 +199,16 @@ protected final void connectToPeer() throws IOException {
// knowing if this is a new transaction function or the answer to say getState

LOGGER.info("making the grpc call");
// for any error - shut everything down
// as this is long lived (well forever) then any completion means something
// has stopped in the peer or the network comms, so also shutdown
final StreamObserver<ChaincodeMessage> requestObserver = chaincodeSupportClient.getStub().register(

new StreamObserver<ChaincodeMessage>() {
@Override
public void onNext(final ChaincodeMessage chaincodeMessage) {
// message off to the ITM...
itm.onChaincodeMessage(chaincodeMessage);
}

@Override
public void onError(final Throwable t) {
LOGGER.severe(
() -> "An error occured on the chaincode stream. Shutting down the chaincode stream."
+ Logging.formatError(t));

LOGGER.severe(() -> "An error occurred on the chaincode stream. Shutting down the chaincode stream." + Logging.formatError(t));
chaincodeSupportClient.shutdown(itm);
}

Expand All @@ -226,7 +218,6 @@ public void onCompleted() {
chaincodeSupportClient.shutdown(itm);
}
}

);

chaincodeSupportClient.start(itm, requestObserver);
Expand Down Expand Up @@ -287,63 +278,43 @@ public void onCompleted() {
}

protected final void initializeLogging() {
// the VM wide formatting string.
System.setProperty("java.util.logging.SimpleFormatter.format",
"%1$tH:%1$tM:%1$tS:%1$tL %4$-7.7s %2$-80.80s %5$s%6$s%n");
System.setProperty("java.util.logging.SimpleFormatter.format", "%1$tH:%1$tM:%1$tS:%1$tL %4$-7.7s %2$-80.80s %5$s%6$s%n");
final Logger rootLogger = Logger.getLogger("");

for (final java.util.logging.Handler handler : rootLogger.getHandlers()) {
Arrays.stream(rootLogger.getHandlers()).forEach(handler -> {
handler.setLevel(ALL);
handler.setFormatter(new SimpleFormatter() {

@Override
public synchronized String format(final LogRecord record) {
return Thread.currentThread() + " " + super.format(record);
}

});
}
});

rootLogger.info("Updated all handlers the format");
// set logging level of chaincode logger
final Level chaincodeLogLevel = mapLevel(System.getenv(CORE_CHAINCODE_LOGGING_LEVEL));

final Package chaincodePackage = this.getClass().getPackage();
if (chaincodePackage != null) {
Logger.getLogger(chaincodePackage.getName()).setLevel(chaincodeLogLevel);
} else {
// If chaincode declared without package, i.e. default package, lets set level
// to root logger
// Chaincode should never be declared without package
Logger.getLogger("").setLevel(chaincodeLogLevel);
}
Optional.ofNullable(this.getClass().getPackage())
.ifPresentOrElse(
pkg -> Logger.getLogger(pkg.getName()).setLevel(chaincodeLogLevel),
() -> Logger.getLogger("").setLevel(chaincodeLogLevel)
);
Copy link
Member

@bestbeforetoday bestbeforetoday Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any real benefit to using an optional to perform an if/else, and I don't think this is available in Java 8

Copy link
Author

@bhaskar-allam bhaskar-allam Oct 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced the Optional usage with a simple null check.
Uses a standard if/else statement, which is more readable and familiar to most Java developers. Now in the latest commit It Is compatible with Java 8 and earlier versions.


// set logging level of shim logger
final Level shimLogLevel = mapLevel(System.getenv(CORE_CHAINCODE_LOGGING_SHIM));
Logger.getLogger(ChaincodeBase.class.getPackage().getName()).setLevel(shimLogLevel);
Logger.getLogger(ContractRouter.class.getPackage().getName()).setLevel(chaincodeLogLevel);

}

private Level mapLevel(final String level) {

if (level != null) {
switch (level.toUpperCase().trim()) {
case "CRITICAL":
case "ERROR":
return Level.SEVERE;
case "WARNING":
case "WARN":
return Level.WARNING;
case "INFO":
return Level.INFO;
case "NOTICE":
return Level.CONFIG;
case "DEBUG":
return Level.FINEST;
default:
break;
}
return switch (level.toUpperCase().trim()) {
case "CRITICAL", "ERROR" -> Level.SEVERE;
case "WARNING", "WARN" -> Level.WARNING;
case "INFO" -> Level.INFO;
case "NOTICE" -> Level.CONFIG;
case "DEBUG" -> Level.FINEST;
default -> Level.INFO;
};
Copy link
Member

@bestbeforetoday bestbeforetoday Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable but the project uses the Java 8 language level, which does not support switch expressions

Copy link
Author

@bhaskar-allam bhaskar-allam Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in latest commit.

}
return Level.INFO;
}
Expand Down Expand Up @@ -442,53 +413,48 @@ protected final void processCommandLineOptions(final String[] args) {
* set fields from env.
*/
public final void processEnvironmentOptions() {
this.id = System.getenv().getOrDefault(CORE_CHAINCODE_ID_NAME, this.id);

Optional.ofNullable(System.getenv(CORE_PEER_ADDRESS))
.ifPresent(address -> {
String[] hostArr = address.split(":");
if (hostArr.length == 2) {
this.port = Integer.parseInt(hostArr[1].trim());
this.host = hostArr[0].trim();
} else {
LOGGER.severe(() -> String.format("peer address argument should be in host:port format, ignoring current %s", address));
}
});
Copy link
Member

@bestbeforetoday bestbeforetoday Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any real benefit to using an optional only to perform a null check

Copy link
Author

@bhaskar-allam bhaskar-allam Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced the Optional.ofNullable().ifPresent() with a simple null check using an if statement. Improved readability by using a more straightforward and familiar Java construct.


if (System.getenv().containsKey(CORE_CHAINCODE_ID_NAME)) {
this.id = System.getenv(CORE_CHAINCODE_ID_NAME);
}
if (System.getenv().containsKey(CORE_PEER_ADDRESS)) {
final String[] hostArr = System.getenv(CORE_PEER_ADDRESS).split(":");
if (hostArr.length == 2) {
this.port = Integer.valueOf(hostArr[1].trim());
this.host = hostArr[0].trim();
} else {
final String msg = String.format(
"peer address argument should be in host:port format, ignoring current %s",
System.getenv(CORE_PEER_ADDRESS));
LOGGER.severe(msg);
}
}

if (System.getenv().containsKey(CHAINCODE_SERVER_ADDRESS)) {
this.chaincodeServerAddress = System.getenv(CHAINCODE_SERVER_ADDRESS);
}

if (System.getenv().containsKey(CORE_PEER_LOCALMSPID)) {
this.localMspId = System.getenv(CORE_PEER_LOCALMSPID);
}
this.chaincodeServerAddress = System.getenv().getOrDefault(CHAINCODE_SERVER_ADDRESS, this.chaincodeServerAddress);
this.localMspId = System.getenv().getOrDefault(CORE_PEER_LOCALMSPID, this.localMspId);

this.tlsEnabled = Boolean.parseBoolean(System.getenv(CORE_PEER_TLS_ENABLED));
if (this.tlsEnabled) {
this.tlsClientRootCertPath = System.getenv(CORE_PEER_TLS_ROOTCERT_FILE);
this.tlsClientKeyPath = System.getenv(ENV_TLS_CLIENT_KEY_PATH);
this.tlsClientCertPath = System.getenv(ENV_TLS_CLIENT_CERT_PATH);

this.tlsClientKeyFile = System.getenv(ENV_TLS_CLIENT_KEY_FILE);
this.tlsClientCertFile = System.getenv(ENV_TLS_CLIENT_CERT_FILE);
}

LOGGER.info("<<<<<<<<<<<<<Environment options>>>>>>>>>>>>");
LOGGER.info("CORE_CHAINCODE_ID_NAME: " + this.id);
LOGGER.info("CORE_PEER_ADDRESS: " + this.host);
LOGGER.info("CORE_PEER_TLS_ENABLED: " + this.tlsEnabled);
LOGGER.info("CORE_PEER_TLS_ROOTCERT_FILE: " + this.tlsClientRootCertPath);
LOGGER.info("CORE_TLS_CLIENT_KEY_PATH: " + this.tlsClientKeyPath);
LOGGER.info("CORE_TLS_CLIENT_CERT_PATH: " + this.tlsClientCertPath);
LOGGER.info("CORE_TLS_CLIENT_KEY_FILE: " + this.tlsClientKeyFile);
LOGGER.info("CORE_TLS_CLIENT_CERT_FILE: " + this.tlsClientCertFile);
LOGGER.info("CORE_PEER_LOCALMSPID: " + this.localMspId);
LOGGER.info("CHAINCODE_SERVER_ADDRESS: " + this.chaincodeServerAddress);
LOGGER.info("LOGLEVEL: " + this.logLevel);
LOGGER.info(() -> String.format("""
<<<<<<<<<<<<<Environment options>>>>>>>>>>>>
CORE_CHAINCODE_ID_NAME: %s
CORE_PEER_ADDRESS: %s
CORE_PEER_TLS_ENABLED: %s
CORE_PEER_TLS_ROOTCERT_FILE: %s
CORE_TLS_CLIENT_KEY_PATH: %s
CORE_TLS_CLIENT_CERT_PATH: %s
CORE_TLS_CLIENT_KEY_FILE: %s
CORE_TLS_CLIENT_CERT_FILE: %s
CORE_PEER_LOCALMSPID: %s
CHAINCODE_SERVER_ADDRESS: %s
LOGLEVEL: %s
""",
Copy link
Member

@bestbeforetoday bestbeforetoday Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Text blocks are not available in Java 8

Copy link
Author

@bhaskar-allam bhaskar-allam Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest Commit is now fully compatible with Java 8.

this.id, this.host, this.tlsEnabled, this.tlsClientRootCertPath,
this.tlsClientKeyPath, this.tlsClientCertPath, this.tlsClientKeyFile,
this.tlsClientCertFile, this.localMspId, this.chaincodeServerAddress, this.logLevel));
}

/**
Expand All @@ -500,26 +466,20 @@ public final void processEnvironmentOptions() {
*/
public Properties getChaincodeConfig() {
if (this.props == null) {
this.props = new Properties();

final ClassLoader cl = this.getClass().getClassLoader();
// determine the location of the properties file to control the metrics etc.

props = new Properties();

try (InputStream inStream = cl.getResourceAsStream("config.props")) {
try (InputStream inStream = this.getClass().getClassLoader().getResourceAsStream("config.props")) {
if (inStream != null) {
props.load(inStream);
}
} catch (final IOException e) {
LOGGER.warning(() -> "Can not open the properties file for input " + Logging.formatError(e));
LOGGER.warning(() -> "Cannot open the properties file for input " + Logging.formatError(e));
}

// will be useful
props.setProperty(CORE_CHAINCODE_ID_NAME, this.id);
props.setProperty(CORE_PEER_ADDRESS, this.host);

LOGGER.info("<<<<<<<<<<<<<Properties options>>>>>>>>>>>>");
LOGGER.info(() -> this.props.toString());
LOGGER.info(() -> "<<<<<<<<<<<<<Properties options>>>>>>>>>>>>\n" + this.props);
}

return this.props;
Expand Down