FWD » Build and Source Control

On my system, ./gradlew core results in the following:

[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/AbstractQuery.java:378: warning: [deprecation] DatabaseStatistics in com.goldencode.p2j.persist has been deprecated
[ant:javac]       if (DatabaseStatistics.isEnabled())
[ant:javac]           ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/AbstractQuery.java:380: warning: [deprecation] DatabaseStatistics in com.goldencode.p2j.persist has been deprecated
[ant:javac]          DatabaseStatistics.get().registerQuery(this);
[ant:javac]          ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/PreselectQuery.java:5096: warning: [deprecation] DatabaseStatistics in com.goldencode.p2j.persist has been deprecated
[ant:javac]          if (DatabaseStatistics.isEnabled())
[ant:javac]              ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/PreselectQuery.java:5099: warning: [deprecation] DatabaseStatistics in com.goldencode.p2j.persist has been deprecated
[ant:javac]             DatabaseStatistics.get().queryExecuted(this, database, hql);
[ant:javac]             ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/RandomAccessQuery.java:4414: warning: [deprecation] DatabaseStatistics in com.goldencode.p2j.persist has been deprecated
[ant:javac]                if (DatabaseStatistics.isEnabled())
[ant:javac]                    ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/RandomAccessQuery.java:4417: warning: [deprecation] DatabaseStatistics in com.goldencode.p2j.persist has been deprecated
[ant:javac]                   DatabaseStatistics.get().queryExecuted(this, database, hql);
[ant:javac]                   ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/admin/client/application/home/console/recordlocks/RecordLocksView.java:323: warning: [deprecation] showFilterDialog(String,HasWidgets,Consumer<String>) in InputDialog has been deprecated
[ant:javac]       inputDialog.showFilterDialog(
[ant:javac]                  ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/admin/client/application/home/print/PrintPreviewView.java:457: warning: [deprecation] showFilterDialog(String,HasWidgets,Consumer<String>) in InputDialog has been deprecated
[ant:javac]       inputDialog.showFilterDialog(
[ant:javac]                  ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/main/StandardServer.java:1290: warning: [deprecation] DatabaseStatistics in com.goldencode.p2j.persist has been deprecated
[ant:javac]                DatabaseStatistics.get();
[ant:javac]                ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/main/StandardServer.java:1303: warning: [deprecation] DatabaseStatistics in com.goldencode.p2j.persist has been deprecated
[ant:javac]             if (DatabaseStatistics.isEnabled())
[ant:javac]                 ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/main/StandardServer.java:1305: warning: [deprecation] DatabaseStatistics in com.goldencode.p2j.persist has been deprecated
[ant:javac]                DatabaseStatistics.get().collectStatistics();
[ant:javac]                ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/main/WebClientsManager.java:242: warning: [deprecation] SslContextFactory() in SslContextFactory has been deprecated
[ant:javac]          SslContextFactory sslContextFactory = new SslContextFactory();
[ant:javac]                                                ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/web/GenericWebServer.java:675: warning: [deprecation] SslContextFactory() in SslContextFactory has been deprecated
[ant:javac]       SslContextFactory sslContextFactory = new SslContextFactory();
[ant:javac]                                             ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/oo/web/WebResponseWriter.java:517: warning: [deprecation] setStatus(int,String) in HttpServletResponse has been deprecated
[ant:javac]                httpResponse.setStatus(response.ref().getStatusCode().intValue(), 
[ant:javac]                            ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/dialect/P2JPostgreSQLDialect.java:2129: warning: [deprecation] toInputStream(String) in IOUtils has been deprecated
[ant:javac]          return IOUtils.toInputStream(data);
[ant:javac]                        ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/id/IdentityPool.java:593: warning: [deprecation] JdbcKeyRetriever in com.goldencode.p2j.persist.id has been deprecated
[ant:javac]                keyRetriever = new JdbcKeyRetriever();
[ant:javac]                                   ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/id/IdentityPool.java:596: warning: [deprecation] StoredProcKeyRetriever in com.goldencode.p2j.persist.id has been deprecated
[ant:javac]                keyRetriever = new StoredProcKeyRetriever();
[ant:javac]                                   ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/orm/UnclosablePreparedStatement.java:311: warning: [deprecation] setUnicodeStream(int,InputStream,int) in PreparedStatement has been deprecated
[ant:javac]    public void setUnicodeStream(int parameterIndex, InputStream x, int length) throws SQLException {
[ant:javac]                ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/orm/UnclosablePreparedStatement.java:312: warning: [deprecation] setUnicodeStream(int,InputStream,int) in PreparedStatement has been deprecated
[ant:javac]       this.target.setUnicodeStream(parameterIndex, x, length);}
[ant:javac]                  ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/web/SecureWebServer.java:178: warning: [deprecation] SslContextFactory() in SslContextFactory has been deprecated
[ant:javac]       SslContextFactory sslContextFactory = new SslContextFactory();
[ant:javac]                                             ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/rest/JsonResponseArguments.java:286: warning: [deprecation] put(String,JsonNode) in ObjectNode has been deprecated
[ant:javac]                   jsonResponse.put(attr, (ObjectNode) val);
[ant:javac]                               ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/util/FileStream.java:1400: warning: ThreadLocalCoders is internal proprietary API and may be removed in a future release
[ant:javac]          decoder = ThreadLocalCoders.decoderFor(sourceCharset);
[ant:javac]                    ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/security/AcmeClient.java:532: warning: [deprecation] TlsSni01Challenge in org.shredzone.acme4j.challenge has been deprecated
[ant:javac]       if (!acceptTlsSniChallenge(((TlsSni01Challenge) challenge).getSubject(),
[ant:javac]                                    ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/security/ManagedWebServer.java:197: warning: [deprecation] SslContextFactory() in SslContextFactory has been deprecated
[ant:javac]       SslContextFactory sslContextFactory = new SslContextFactory();
[ant:javac]                                             ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/driver/swing/SwingKeyboardReader.java:190: warning: [deprecation] setModifiers(int) in KeyEvent has been deprecated
[ant:javac]          last.setModifiers(last.getModifiers() | InputEvent.SHIFT_MASK);
[ant:javac]              ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/driver/swing/SwingKeyboardReader.java:194: warning: [deprecation] setModifiers(int) in KeyEvent has been deprecated
[ant:javac]          last.setModifiers(last.getModifiers() | InputEvent.ALT_MASK);
[ant:javac]              ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/driver/swing/SwingKeyboardReader.java:198: warning: [deprecation] setModifiers(int) in KeyEvent has been deprecated
[ant:javac]          last.setModifiers(last.getModifiers() | InputEvent.CTRL_MASK);
[ant:javac]              ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/gui/driver/web/GuiWebDriver.java:2725: warning: [deprecation] getTextWidth(String,int) in GuiWebSocket has been deprecated
[ant:javac]             scaleX = (double)websock.getTextWidth(TEXT_SAMPLE_MONO, fd.font)/
[ant:javac]                                     ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/gui/driver/web/GuiWebDriver.java:2731: warning: [deprecation] getTextWidth(String,int) in GuiWebSocket has been deprecated
[ant:javac]             wRes = websock.getTextWidth(text, fd.font);
[ant:javac]                           ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/gui/driver/web/GuiWebDriver.java:2736: warning: [deprecation] getTextWidth(String,int) in GuiWebSocket has been deprecated
[ant:javac]             scaleX = (double)websock.getTextWidth(TEXT_SAMPLE, fd.font)/
[ant:javac]                                     ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/xml/XDocumentImpl.java:1495: warning: [deprecation] setEncoding(String) in CoreDocumentImpl has been deprecated
[ant:javac]       ((DocumentImpl) xdoc).setEncoding(encodingStr);
[ant:javac]                            ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/util/DatabaseDumpChecker.java:216: warning: ThreadLocalCoders is internal proprietary API and may be removed in a future release
[ant:javac]       CharsetDecoder decoder = ThreadLocalCoders.decoderFor(sourceCS);
[ant:javac]                                ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/util/DatabaseDumpChecker.java:220: warning: ThreadLocalCoders is internal proprietary API and may be removed in a future release
[ant:javac]       CharsetEncoder encoder = ThreadLocalCoders.encoderFor(targetCS);
[ant:javac]                                ^
[ant:javac] Note: Some input files use unchecked or unsafe operations.
[ant:javac] Note: Recompile with -Xlint:unchecked for details.
[ant:javac] 33 warnings
> Task :ant-compile

> Task :ant-aspectj
[ant:iajc] warning at /home/ges/projects/3821c/build/classes!com/goldencode/util/LoggingAspect.class::0 advice defined in com.goldencode.util.LoggingAspect has not been applied [Xlint:adviceDidNotMatch]
[ant:iajc] warning at /home/ges/projects/3821c/build/classes!com/goldencode/p2j/aspects/LoadLegacyClassAspect.class::0 advice defined in com.goldencode.p2j.aspects.LoadLegacyClassAspect has not been applied [Xlint:adviceDidNotMatch]

> Task :ant-jar
> Task :ant-native_prepare

> Task :ant-native
[ant:echo] 
[ant:javadoc] javadoc: warning - Multiple sources of package comments found for package "javax.servlet.http" 
[ant:javadoc] javadoc: warning - Multiple sources of package comments found for package "javax.servlet" 

I'm not sure everyone sees the AspectJ warnings, but I think everyone sees the others.

Either way, please eliminate all of these warnings, including the deprecated notices.

I understand the idea of using @Deprecated. It might be OK, if we only left these warnings for a short period of time and they always got cleaned up (fairly quickly). That is not ever what happens. Instead, there is an ever growing, never shrinking list of deprecated warnings in the build. There is a real cost to that list. When there are errors, one must search carefully through the output to find the real problems. We are going to eliminate all of them. Yes, all of them.

• If the @Deprecated code is in FWD, we will discuss the approach. Best case, we rework the call sites to eliminate the usage and then actually delete the @Deprecated code. Worst case is we remove the @Deprecated annotation and move on with our lives, guilt-free.
• If the @Deprecated is outside of FWD, we will need to evaluate the usage. I think we eliminate the usage or we remap the usage to something not @Deprecated.

In regard to J2SE @Deprecated code, please note that in #5567 we will be moving to Java 11. That shift may require that we eliminate some of this usage. To the degree that it is easier to eliminate that usage in #5567, I am OK deferring that work briefly. Otherwise, it all goes.

• The first half a dozen warnings are generated by DatabaseStatistics. It is "dangling" since we switch away from Hibernate. Although I have never seen it in action, the result might be useful sometime in the future. For a minimal insight into database activity, have the SQLStatementLogger, but it is very verbose while activated and does not gather statistical data at any level. We could just drop it or, if we decide to implement it with the new ORM, a new layer needs to be created to do the counting and timings. Some time ago we wrote temporary routines for timing some parts of FWD so this is definitely useful. I think the class (and its companions QueryStatistics, SecondLevelCacheStatistics, Statistics should be dropped and equivalent code written in SQLQuery firs time when needed.
• JdbcKeyRetriever and StoredProcKeyRetriever: to be removed along with KeyRetriever interface.
• UnclosablePreparedStatement: this class inherits the setUnicodeStream() from its interface. I do not know how/if we can avoid this warning.

Eric, do you agree ?

Ovidiu Maxiniuc wrote:

• The first half a dozen warnings are generated by DatabaseStatistics. It is "dangling" since we switch away from Hibernate. Although I have never seen it in action, the result might be useful sometime in the future. For a minimal insight into database activity, have the SQLStatementLogger, but it is very verbose while activated and does not gather statistical data at any level. We could just drop it or, if we decide to implement it with the new ORM, a new layer needs to be created to do the counting and timings. Some time ago we wrote temporary routines for timing some parts of FWD so this is definitely useful. I think the class (and its companions QueryStatistics, SecondLevelCacheStatistics, Statistics should be dropped and equivalent code written in SQLQuery firs time when needed.

Although we did not prioritize this tooling when removing Hibernate, I don't want to throw this code away. We have other tools which can help with this kind of diagnosis (e.g. pgBadger, YourKit, ...), but I found the output of this tooling pretty useful for tracking down problem queries. However, I don't want to spend any time reimplementing this in the near term to work with the new ORM code. It is a longer term TODO and any new implementation would be different in several ways. I'm not sure I want to replicate what Hibernate was doing; I just used that functionality because it already was there in Hibernate, but it was not ideal. Let's remove the @Deprecated tags to eliminate the compile warnings, and put TODO comments in the code instead, so we know not to throw away the code.

• JdbcKeyRetriever and StoredProcKeyRetriever: to be removed along with KeyRetriever interface.

Yes, I don't see us using these approaches in the future. They can be removed.

• UnclosablePreparedStatement: this class inherits the setUnicodeStream() from its interface. I do not know how/if we can avoid this warning.

Won't the @SuppressWarnings annotation silence the compile warning?

ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/driver/swing/SwingKeyboardReader.java:190: warning: [deprecation] setModifiers(int) in KeyEvent has been deprecated
[ant:javac]          last.setModifiers(last.getModifiers() | InputEvent.SHIFT_MASK);
[ant:javac]              ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/driver/swing/SwingKeyboardReader.java:194: warning: [deprecation] setModifiers(int) in KeyEvent has been deprecated
[ant:javac]          last.setModifiers(last.getModifiers() | InputEvent.ALT_MASK);
[ant:javac]              ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/driver/swing/SwingKeyboardReader.java:198: warning: [deprecation] setModifiers(int) in KeyEvent has been deprecated
[ant:javac]          last.setModifiers(last.getModifiers() | InputEvent.CTRL_MASK);

KeyEvent in place modification is depracated since JDK1.1.4, wrote replacement code - in case modifiers are needed KeyEvent is cloned with needed modifiers and put into front of the queue keyDownEvents in WinKeyboardReader

----

ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/gui/driver/web/GuiWebDriver.java:2725: warning: [deprecation] getTextWidth(String,int) in GuiWebSocket has been deprecated
[ant:javac]             scaleX = (double)websock.getTextWidth(TEXT_SAMPLE_MONO, fd.font)/
[ant:javac]                                     ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/gui/driver/web/GuiWebDriver.java:2731: warning: [deprecation] getTextWidth(String,int) in GuiWebSocket has been deprecated
[ant:javac]             wRes = websock.getTextWidth(text, fd.font);
[ant:javac]                           ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/ui/client/gui/driver/web/GuiWebDriver.java:2736: warning: [deprecation] getTextWidth(String,int) in GuiWebSocket has been deprecated
[ant:javac]             scaleX = (double)websock.getTextWidth(TEXT_SAMPLE, fd.font)/

[ant:javac] /home/tjd/projects/p2j_3821c/src/com/goldencode/p2j/persist/dialect/P2JPostgreSQLDialect.java:2129: warning: [deprecation] toInputStream(String) in IOUtils has been deprecated
[ant:javac]          return IOUtils.toInputStream(data);

-------

[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/orm/UnclosablePreparedStatement.java:311: warning: [deprecation] setUnicodeStream(int,InputStream,int) in PreparedStatement has been deprecated
[ant:javac]    public void setUnicodeStream(int parameterIndex, InputStream x, int length) throws SQLException {
[ant:javac]                ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/persist/orm/UnclosablePreparedStatement.java:312: warning: [deprecation] setUnicodeStream(int,InputStream,int) in PreparedStatement has been deprecated
[ant:javac]       this.target.setUnicodeStream(parameterIndex, x, length);}

[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/rest/JsonResponseArguments.java:286: warning: [deprecation] put(String,JsonNode) in ObjectNode has been deprecated
[ant:javac]                   jsonResponse.put(attr, (ObjectNode) val);

----

[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/util/FileStream.java:1400: warning: ThreadLocalCoders is internal proprietary API and may be removed in a future release
[ant:javac]          decoder = ThreadLocalCoders.decoderFor(sourceCharset);
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/util/DatabaseDumpChecker.java:216: warning: ThreadLocalCoders is internal proprietary API and may be removed in a future release
[ant:javac]       CharsetDecoder decoder = ThreadLocalCoders.decoderFor(sourceCS);
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/util/DatabaseDumpChecker.java:220: warning: ThreadLocalCoders is internal proprietary API and may be removed in a future release
[ant:javac]       CharsetEncoder encoder = ThreadLocalCoders.encoderFor(targetCS);

replaced ThreadLocalCoders.decoderFor/encoderFor with non-cached version Charset.decoderFor/encoderFor, performance may be reduced.

[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/security/AcmeClient.java:532: warning: [deprecation] TlsSni01Challenge in org.shredzone.acme4j.challenge has been deprecated
[ant:javac]       if (!acceptTlsSniChallenge(((TlsSni01Challenge) challenge).getSubject(),

TLS-SNI-01
This challenge was defined in draft versions of ACME. It did a TLS handshake on port 443 and sent a specific SNI header, looking for certificate that contained the token. It was disabled in March 2019 because it was not secure enough.

TlsSni01Challenge is unsupported since 2019, it must to be migrated to other method. I guess AcmeClient does not work at the moment. New ticket is needed for this. Not changing the code at moment.
-----

[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/oo/web/WebResponseWriter.java:517: warning: [deprecation] setStatus(int,String) in HttpServletResponse has been deprecated
[ant:javac]                httpResponse.setStatus(response.ref().getStatusCode().intValue(), 

According to related TODO I presume this method's logic needs to be moved to separate class,

    * TOOD: This method should not be here, InputDialog class should not handle any specific application logic.

Is there a place to put such a class ? Maybe package com.goldencode.p2j.admin.client.application.home ?

Hynek: What is your recommendation?

TlsSni01Challenge is unsupported since 2019, it must to be migrated to other method. I guess AcmeClient does not work at the moment. New ticket is needed for this.

Agreed. Please do the following:

• Create a new task for the rework in the Runtime Infrastructure project (sub-project of FWD).
• Modify AcmeClient to report that it is non-functional and to exit.
• Comment out the usage that causes the javac warning.

Its not possible to directly replace this method with other:

Add comments in that code with your analysis so that future editors have a good starting point.

Remote GuiWebSocket.getTextWidth is deprecated for single text sample

I think we can just remove the deprecated annotation here.

Replaced with IOUtils.toInputStream(data,Charset.defaultCharset()), note that serialization of CLOBs depends on platform charset, which may cause problems on migration or in heterogeneous environment.

Ovidiu/Eric: Please advise on the proper approach for handling this.

Greg Shah wrote:

Replaced with IOUtils.toInputStream(data,Charset.defaultCharset()), note that serialization of CLOBs depends on platform charset, which may cause problems on migration or in heterogeneous environment.

Ovidiu/Eric: Please advise on the proper approach for handling this.

First, I suggest to replace the code with

        return new ByteArrayInputStream(data.getBytes(Charset.defaultCharset()));

Secondly, it's clear that the current implementation has a problem because if some non-supported content is put in a clob which will go to a database with charset constraints errors will occur. I do not know of any for the moment, but this is just because the content 'behaved' nicely and fit to target CP.

• store the CP/Encoding in the dialect and configure the CP/Charset when building the FWDClob object or
• maybe we can obtain it from the Connection parameter the clobCreator() method receives.

The Charset will eventually be saved in the new FWDClob object itself.

[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/admin/client/application/home/console/recordlocks/RecordLocksView.java:323: warning: [deprecation] showFilterDialog(String,HasWidgets,Consumer<String>) in InputDialog has been deprecated
[ant:javac]       inputDialog.showFilterDialog(
[ant:javac]                  ^
[ant:javac] /home/ges/projects/3821c/src/com/goldencode/p2j/admin/client/application/home/print/PrintPreviewView.java:457: warning: [deprecation] showFilterDialog(String,HasWidgets,Consumer<String>) in InputDialog has been deprecated
[ant:javac]       inputDialog.showFilterDialog(
[ant:javac]                  ^

According to related TODO I presume this method's logic needs to be moved to separate class,

Removed showFilterDialog from InputDialog, implemented Filter Dialog for RecordLocksView in similar way as its done for session view, working on the same approach for PrintPreviewView

Sergey: Please review the admin client changes.
Ovidiu: Please review the persistence/database stats/database dump checker changes.
Hynek/Constantin: Please review the rest of the changes.

Tomasz: Test Hotel GUI and ChUI Regression Testing. Please note that we have not run ChUI Regression Testing regression testing in a while and there were some regressions already in 3821c. So I think you'll have to compare a 3821c regression run against the 3821c + your diffs to see if it is OK.

Greg Shah wrote:

Ovidiu: Please review the persistence/database stats/database dump checker changes.

• P2JPostgreSQLDialect: javadoc header: IOUtils is not deprecated, just not an accepted library for FWD project.
• QueryStatistics, SecondLevelCacheStatistics, Statistics, UnclosablePreparedStatement, SwingKeyboardReader, GuiWebSocket, FileStream, XDocumentImpl, DatabaseDumpChecker: the header entry is required even if no actual code was changed.
• IdentityPool: maybe rephrase a bit the warning message.

Alternatively one of the UnimplementedFeature methods could be invoked at the places where a TODO replaced the deprecation warning. I am thinking here primarily of IdentityPool, but also DatabaseStatistics. In the later, the notification is probably better only if the module is enabled.

Greg Shah wrote:

Hynek/Constantin: Please review the rest of the changes.

• AcmeClient.java - you've disabled this completely. Is this class really no longer working with Let's Encrypt? If so, we need to fix it, not disable it.
• FileStream.setConvertSource - we should not leave TODOs in place...
  

           //TODO: cache decoders per thread
           decoder = sourceCharset.newDecoder();
  

  
  if we need to cache these, then create a ContextLocal WorkArea class, with a 'decoderCache' map and cache it there. In FWD, ThreadLocal can be used only in limited places.
• SecureWebServer - remove the explicit import org.eclipse.jetty.util.ssl.SslContextFactory.Server;.
• XDocumentImpl.setEncoding - although setXmlEncoding does the same as setEncoding, the parameter really needs to be 'encodingStr' and not 'javaEncoding'.
• GuiWebSocket.getTextWidth and others - these were deprecated in H027 CA 20171204, with bzr rev 11213.1.24:
  

  **     CA  20171204 Allow the Web driver to measure text on the client-side, via a AWT font; this
  **                  avoids expensive JS calls to measure the text.  For this reason, the text
  **                  related APIs were deprecated.
  

  
  The point was to change the FWD code and remove these calls, so we can avoid measuring text on the JS side (as this is expensive), and do everything on the Java client side. But looks like getTextWidth calls could not be removed safely (yet). I think the Deprecated annotation should be removed.

Is anyone able to build and run ChUI regression tests/p2j 3821c integration tests ?
I am getting errors since last week.
I am running run_regression.sh system-password build

Standard scripts require a stable p2j branch - is there a way to run it against selected revision ?

Recent FWD changes related to schema processing have caused some breakage. Roger is working on moving to a more standard project structure in #6213. During that work the issues are being identified and resolved. He is close to having a working build.

As code lives on there are new warnings to be removed like:

[ant:javac] /home/tjd/projects/p2j_3821c/src/com/goldencode/p2j/ui/BaseEntity.java:551: warning: [deprecation] flushWidgetAttrs() in GenericWidget has been deprecated
[ant:javac]       flushWidgetAttrs();
[ant:javac] /home/tjd/projects/p2j_3821c/src/com/goldencode/p2j/ui/BrowseWidget.java:8790: warning: [deprecation] pushWidgetAttr(String[],Object[]) in GenericWidget has been deprecated
[ant:javac]          column.pushWidgetAttr(new String[] {"dataType", "format", "rightAligned"},

I assume we would remove the @Deprecated.

Hynek/Ovidiu: Please review 5567a.

• general: each file's header must be updated with a new historical entry. The (c) year must also be updated.
• there are occurrences of hard TAB (0x09) (build.xml). These must be replaced with 3 spaces (0x20);
• there are some imports of java.lang.reflect.InvocationTargetException. The preferred way is using bulk:
  

  import java.lang.reflect.*;

  Use the single class variant only in case there are conflicts.
• P2JLocaleHelper.java: constructs like:
  

  Integer cp = new Integer(codePoint);
  return String.format("<U%04X>", new Object[] { cp });

  
  should be rewritten to take advantage of the autoboxing and vararg parameter calls (already available in Java 5), no need to use the additional objects anymore:
  

  return String.format("<U%04X>", codePoint);

  There are other similar occurrences like XmlFilePlugin:848 and 1035, Mapper:589-614, ProxyAssembler:593, EscapeControlCharsReader:114 and 178, DirectoryService, RemapTestDriver1, Attribute:535-541, RamRemapper, ShadowRemapper, StreamDaemon, hql.g, fql.g, SecurityAdmin, and many others. Generally, instead of <JavaWrapper>.valueOf(javaScalar), use the autoboxing feature. In some rare cases (VectorGraphicsHelper:248-249, Tracer:) a cast to desired type can be necessary;
• XmlAst: 107-140, 692: use the auto boxing. Special case here since the values are processed below in pair, it seems logical to add them to map on two columns so that the reader can easily understand the logic of the code, like this:
  

          ATTRIBUTE_NODE,       "ATTRIBUTE_NODE",
          CDATA_SECTION_NODE,   "CDATA_SECTION_NODE",

• AstManagerPlugin:396, BaseDataType:1144, DataObjectFactory:160: the list of exceptions should be split with one item per line if too long (110 chars). Please column align the | in other places, too;
• LdapMapGen:605: nice addition of generics. Use the diamond notation <> to simplify the code and let the compiler infer the types for constructor;
• BinaryData:563: what about rewriting the line as: int result = Byte.compare(value1[i], value2[i]); ?
• SecondLevelCacheStatistics, Statistics, QueryStatistics: The addition of the history and copyright header is correct, although I am not sure about the life span of this file. Just move them above the package declaration and add back the class-level javadoc and set the copyright year to "2020-2022";
• JsonObject:1602, StreamConnector:232, FontTable:1777-1779: I think these lines can be safely removed;
• LexerDumpFilter:289: why not calling super.finalize(); ? It is a good practice. Even if momentarily it will just call the empty method of Object, its super could be altered to implement this method, too. Not calling the super method could cause the resources of the super class to remained un-freed (is that a word?);
• GuiWebDriver:1560-1582: are the new masks correct? They seem to have different values though (BUTTON1_MASK=16 but BUTTON1_DOWN_MASK=1024);
• the GC code guidelines state that the operands and the operators are separated by a space (DatabaseDumpChecker:215, 217). Also one space is used between a statement keyword (if, while, try) and the opening parenthesis (FileDialogGuiImpl:1519, 2889, NormalizePath:150).

300+ files! Phew!

Greg Shah wrote:

Hynek/Ovidiu: Please review 5567a.

I meant that #6208 is ready and it is integrated into 5567a, but 5567a is not ready yet.
All the changes to #6208 are in the attached patches, as there is no dedicated branch.
I will copy Ovidiu's comments as intermediate review of 5567a as they better fit there.
Sorry for misunderstanding.

I noticed that changes for #6208 are already in 5567a. They were the only files with history entry added :). I saw nothing wrong with them, except for minor issues in SecondLevelCacheStatistics, Statistics, QueryStatistics, which I added in my review. TBH, I am eager to see these in changes in main branch. Having all the changes of 5567a merged in would be super. Nice work so far!

Hynek/Ovidiu: Please review the fix_all_compile_warnings_6208_v4.diff.

If the #6208 changes are ready for merge and the risk is low, then we can merge those changes into 3821c now and Tomasz can continue working on 5567a for the Java 11 support.

Greg Shah wrote:

Hynek/Ovidiu: Please review the fix_all_compile_warnings_6208_v4.diff.

I am OK with the proposed code, just one thing I have just noticed: in P2JPostgreSQLDialect.java, we have the implementation of Clob.getAsciiStream(). The updated code is:

return new ByteArrayInputStream(data.getBytes(Charset.defaultCharset()));

return new ByteArrayInputStream(data.getBytes(StandardCharsets.US_ASCII));

Retrieves the CLOB value designated by this Clob object as an ascii stream.

However, it's probably best, for the moment, to keep the current implementation as it proved to be safe. Or, at least, nobody experienced/observed/reported problems related to this issue. I think we should let a TODO: comment about it.

it's probably best, for the moment, to keep the current implementation as it proved to be safe

Agreed, a TODO is fine for this issue.

Tomasz: Go ahead and merge the compile warnings fixes to 3821c.

Merged into 3821c rev 13856

I see these issues that remain:

> Task :ant-aspectj
[ant:iajc] warning at /home/ges/projects/3821c/build/classes!com/goldencode/util/LoggingAspect.class::0 advice defined in com.goldencode.util.LoggingAspect has not been applied [Xlint:adviceDidNotMatch]
[ant:iajc] warning at /home/ges/projects/3821c/build/classes!com/goldencode/p2j/aspects/LoadLegacyClassAspect.class::0 advice defined in com.goldencode.p2j.aspects.LoadLegacyClassAspect has not been applied [Xlint:adviceDidNotMatch]

...

> Task :ant-native
[ant:echo] 
[ant:javadoc] javadoc: warning - Multiple sources of package comments found for package "javax.servlet.http" 
[ant:javadoc] javadoc: warning - Multiple sources of package comments found for package "javax.servlet" 

Am I the only one that sees the AspectJ issue?

I think everyone sees the javadoc issue.

I resolved the LoggingAspect warning in 3821c revision 13857.

I think we can close this now.