Skip to content
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

fix #78 use space and gql are executed together incorrect in 3.3.0 #87

Merged
merged 4 commits into from
Nov 25, 2022
Merged

fix #78 use space and gql are executed together incorrect in 3.3.0 #87

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
20cc5e8
fix #78 use space and gql are executed together incorrect in 3.3.0
CorvusYe Nov 7, 2022
53bd4e2
improvement: change current session space from ResultSet instead of R…
CorvusYe Nov 8, 2022
583662b
Merge branch 'nebula-contrib:master' into master
CorvusYe Nov 18, 2022
675192f
lint
CorvusYe Nov 18, 2022
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
25 changes: 19 additions & 6 deletions src/main/java/org/nebula/contrib/ngbatis/proxy/MapperProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
//
// This source code is licensed under Apache 2.0 License.

import static org.apache.commons.lang3.ObjectUtils.isEmpty;
import static org.nebula.contrib.ngbatis.models.ClassModel.PROXY_SUFFIX;

import com.vesoft.nebula.client.graph.data.ResultSet;
import com.vesoft.nebula.client.graph.exception.IOErrorException;
import com.vesoft.nebula.client.graph.net.Session;
import java.io.IOException;
import java.lang.reflect.Method;
Expand Down Expand Up @@ -211,6 +213,7 @@ public static ResultSet executeWithParameter(ClassModel cm, MethodModel mm, Stri
String proxyClass = null;
String proxyMethod = null;
String localSessionSpace = null;
String autoSwitch = null;
try {
localSession = ENV.getDispatcher().poll();
if (log.isDebugEnabled()) {
Expand All @@ -221,7 +224,9 @@ public static ResultSet executeWithParameter(ClassModel cm, MethodModel mm, Stri
}

String currentSpace = getSpace(cm, mm);
gql = qlWithSpace(localSession, gql, currentSpace);
String[] qlAndSpace = qlWithSpace(localSession, gql, currentSpace);
gql = qlAndSpace[1];
autoSwitch = qlAndSpace[0] == null ? "" : qlAndSpace[0];
session = localSession.getSession();
result = session.executeWithParameter(gql, params);
if (result.isSucceeded()) {
Expand All @@ -236,23 +241,31 @@ public static ResultSet executeWithParameter(ClassModel cm, MethodModel mm, Stri
if (log.isDebugEnabled()) {
log.debug("\n\t- proxyMethod: {}#{}"
+ "\n\t- session space: {}"
+ (isEmpty(autoSwitch) ? "{}" : "\n\t- auto switch to: {}")
+ "\n\t- nGql：{}"
+ "\n\t- params: {}"
+ "\n\t- result：{}",
proxyClass, proxyMethod, localSessionSpace, gql, params, result);
proxyClass, proxyMethod, localSessionSpace, autoSwitch, gql, params, result);
}
if (localSession != null) {
ENV.getDispatcher().offer(localSession);
}
}
}

private static String qlWithSpace(LocalSession localSession, String gql, String currentSpace) {
private static String[] qlWithSpace(LocalSession localSession, String gql, String currentSpace)
throws IOErrorException {
String[] qlAndSpace = new String[2];
gql = gql.trim();
String sessionSpace = localSession.getCurrentSpace();
return Objects.equals(sessionSpace, currentSpace)
? String.format("\n\t\t%s", gql)
: String.format("USE %s;\n\t\t%s", currentSpace, gql);
boolean sameSpace = Objects.equals(sessionSpace, currentSpace);
if (!sameSpace) {
qlAndSpace[0] = currentSpace;
Session session = localSession.getSession();
session.execute(String.format("USE %s", currentSpace));
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer to execute USE space after each request. In resultSet of execution's response, there's space name for current session, and we can decide whether need to change the space back.

If execute USE space before request, there will be some useless operations, ge:
Local session's current space is test, and user's request is "use test1; show hosts; use test" , and there is no need to pre-execute a use test1 operation.

Copy link
Collaborator Author

@CorvusYe CorvusYe Nov 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

In this case, you are right.

And we should consider another situation, for example there are two DAO:

<mapper namespace="Test1Dao" space="test1">

    <select id="useCurrentSpace" >
         RETURN 'useCurrentSpace';
    </select>

    <select id="useTestSpace" space="test">
         RETURN 'useTestSpace';
    </select>

    <select id="useSpaceInner">
         USE test;
         RETURN 'useSpaceInner';
    </select>
    
</mapper>


<mapper namespace="DefaultSpaceInYmlDao"> <!-- default space is `test`-->
    
    <select id="normalInvoke">
         RETURN 'normalInvoke';
    </select>

</mapper>
    @Autowired private Test1Dao test1Dao;
    @Autowired private DefaultSpaceInYmlDao defaultSpaceDao;
    
    public void biz() {
        test1Dao.useCurrentSpace(); // when this session is use in first time, space is null, so need to specify the space
        // USE test1;
        // RETURN 'useCurrentSpace';

        test1Dao.useCurrentSpace(); // local session keep the last space, so this time is unnecessary to switch
        // RETURN 'useCurrentSpace';

        test1Dao.useTestSpace(); // catching the space is difference from local session.
        // USE test;
        // RETURN 'useTestSpace';

        defaultSpaceDao.normalInvoke(); 
        // RETURN 'normalInvoke';

        test1Dao.useTestSpace();
        // RETURN 'useTestSpace';

        test1Dao.useSpaceInner(); // This usage #78 may happend again, because we can not control how ngql will be written
        // USE test; RETURN 'useSpaceInner'; 
    }

Let's assume that only one session is in use.
In this case 6 times invoked, we costs 8 times request. not 12 times.

Ngbatis recommends declaring spaces in xml tags.

And you reminded me that we can get current session space from ResultSet insteads of RegExp.

}
qlAndSpace[1] = String.format("\n\t\t%s", gql);
return qlAndSpace;
}

private static void setNewSpace(LocalSession localSession, String gql, String currentSpace) {
Expand Down