Code review of GIC phase-3

e15d5605 commented 2 years ago

GICによってGlycanBuilderのphase-3の更新が行われた\ GICの変更はGICDevelopブランチにプッシュされているため、まずはGICDevelopでの確認を行う\ GICDevelopでの確認が済み次第、Developの更新内容と照らし合わせて変更内容に衝突が発生していないか検証を行う\ Developブランチでの問題が見られなかったらmasterへマージしてv.1.16.0としてリリースする

大まかな作業の流れ

GICDevelopの確認
GICDevelopをDevelopと比較・検証
GICDevelopをDevelopへマージ
Developを検証
Developをmasterへマージ
v1.16.0としてリリース

e15d5605 commented 2 years ago

GlycanDocument

exportFromStructuresという関数が実装されていた\ 引数に該当するParserを取得して、それぞれのGlycanからテキストを生成している\ BuilderWorkbenchあたりに同様の関数が実装されていた（はず）のため目的は不明…要確認

public ArrayList<String> exportFromStructures(Collection<Glycan> a_lstGlycan, String format) throws Exception {

確認したところ、元々の関数の実装方法に問題があるように感じられる

オリジナルオリジナルのexportFromStructuresは、Glycanから変換されたテキストをグローバル変数のlst_encodeに格納している\ exportFromStructuresを実行後、lst_encodeのgetterで糖鎖テキストを取得する

/**
 * Encode the structures into a string using the specified format:
 */
public boolean exportFromStructure(Collection<Glycan> a_lstGlycan, String format) {
    try{
        String str_encode = "";
        GlycanParser parser = GlycanParserFactory.getParser(format);
        str_encode = toString(a_lstGlycan, parser);
        if(str_encode.equals("")) throw new Exception("Invalid output string");

        for(String s : str_encode.split(";")) this.lst_encode.addLast(s);
        return true;
    } catch (Exception e) {
        LogUtils.report(e);
        return false;
    }
}

GIC GlycanStructureAndChangeUserのgetAccessionIdorHashKeyでテキスト（WURCS）の取得を行っている\ GlycanDocumentのexportFromStructuresでテキストを取得したいが、戻り値がbooleanのため使い勝手がよくなかったと思われる\ GIC側の関数は配列に変換されたWURCSを保管して戻り値としている

public void getAccessionIdorHashKey(String env, String keyPath, String reg_url)
    ...
    try {
        ArrayList<String> sequences = getSequences();
    ...
}

/**
* Get sequence to get Accession Id or HashKey
*/
private ArrayList<String> getSequences() throws Exception {
    ArrayList<String> seq_strArr = new ArrayList<>();;
    if (this.theDoc.getStructures().size() > 0) {
        seq_strArr = this.theDoc.exportFromStructures(this.theDoc.getStructures(), "wurcs2");
    }

    return seq_strArr;
}

/**
 *  Encode all structures and return specific format
 *  @author GIC 20210105
 */
public ArrayList<String> exportFromStructures(Collection<Glycan> a_lstGlycan, String format) throws Exception {
    ArrayList<String> arr = new ArrayList<>();
    GlycanParser parser = GlycanParserFactory.getParser(format); 
    for(Glycan glycan : a_lstGlycan) {
        LinkedList<Glycan> colGlycan = new LinkedList<Glycan>();
        colGlycan.add(glycan);
        arr.add(toString(colGlycan,parser).toString());
    } 
    return arr; 
}

e15d5605 commented 2 years ago

新規ファイル

新規ファイル一覧

GlycanStructureAndChangeUser.java
GlytoucanIDList.java
SelectAPIDialog.java
UserLoginDialog.java

これらはGIC側が作成した新規ファイルのため、GlycanBuilder2の既存の処理へ直接影響はしないと思われる

e15d5605 commented 2 years ago

CanvasCommand

createGlyTouCanRibbonBandという関数が実装されている\ GlycoWorkbenchで使用する以下のコマンドを生成するための関数である

CanvasAction

GlycoWorkbenchのGlyTouCanタブのコマンドをクリックした時のアクションを定義している

CanvasActionDescriptor

GlycoWorkbenchのGlyTouCanタブのコマンドをクリックした時のアクションのIDを定義している

SELECTAPIDIALOG("selectapidialog"), 
GLYCANIDLIST("glycanidlist"),       
CHANGEUSER("edituser"),

e15d5605 commented 2 years ago

GlycanCanvas

新規の関数

/**
* Get GlyTouCan Ribbon
* @author GIC 20211217
*/
public RibbonTask getTheGlyTouCanRibbon() {
    return theGlyTouCanRibbon;
}

/**
 * Create GlyTouCan Ribbon
 * @author GIC 20211219
 */
private RibbonTask createGlyTouCanRibbonBand() {
    return this.a_oCommand.createGlyTouCanRibbonBand(getTheActionManager(), this);
}

/**
 * Show Environment Select API Dialog
 * @author GIC 20211219
 */
private void onSelectAPIDialog() {
    if (this.theDoc.getStructures().size() == 0) {
        JOptionPane.showMessageDialog(null, "Glycan Structure is empty", "Error in GlycanBuilder2",
            JOptionPane.ERROR_MESSAGE);
    } else {
        theStructure.onSelectAPIDialog(theParent,this.theDoc);
    }
}

/**
 *  Show Change User Dialog
 *  @author GIC 20211220
 */
private void changeUser() {
    try {
        theStructure.changeUser();
    } catch (ParseException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
}

actionPerformedに以下の処理が追加されていた\ ここではUIのコマンドを選択した際の内容に応じた機能が呼び出される

/****************************GIC added 20211220**********************/
//Send glycan structure
if(a_enumAction.equals(CanvasActionDescriptor.SELECTAPIDIALOG)) onSelectAPIDialog();

//Show GlycanIDList
if(a_enumAction.equals(CanvasActionDescriptor.GLYCANIDLIST)) {
    GlytoucanIDList.showGlycanIdList(theParent); 
}   

//change user
if(a_enumAction.equals(CanvasActionDescriptor.CHANGEUSER)) changeUser();
/*********************************************************************/

e15d5605 commented 2 years ago

所見

全体を確認した結果、GWBのGlyTouCanコマンドの生成や、そのコマンドを実行する関数が追加されている\ GlyTouCanコマンドを実行する関数は独自のファイルに実装されているため、GB2からは参照されない\ 一方で、CanvasCommandやCanvasAction、CanvasActionDescriptorはGB2とGWBの双方で参照する\ DevelopでもGlycanCanvas、CanvasAction、CanvasActionDescriptorを変更している（https://github.com/glycoinfo/GlycanBuilder2/commit/953e79fdd9aab2bb9513b64124083b77c546fb28 , https://github.com/glycoinfo/GlycanBuilder2/commit/9ef43dfc8c5dac75cd0dc7f6b888a0f3393ab565 ） GICの変更との衝突が予想される\

この変更は、GWBの糖鎖テンプレートギャラリーが表示されていない問題（GWB2#3）を解決するための作業である

以上のことから、GICDevelopをmasterにマージして（1.16.0）として公開が可能かを検証する

e15d5605 commented 2 years ago

master vs GICDevelop

IntelliJのgitブランチ比較機能を利用して変更状態を確認する\ 緑がGICDevelopユニークの変更、青がお互いのブランチのどちらか（あるいは両方）で変更が加えられたことを示す

GlycanCanvasの例を以下に示す\ masterの変更は青色、GICDevelopの変更は緑でハイライトされる

masterのみ変更
GICDevelopのみ変更

ハイライトが混ざっていなければコンフリクトは起こらないと思われる\ 結果としてソースコードにコンフリクトが発生する可能性のある箇所は見受けられなかった\ .gitignoreとeclipseの環境設定ファイルにコンフリクトが見られたため、GICDevelopの変更を上書きしてmasterへマージした

e15d5605 commented 2 years ago

v1.16.0としてpush

glycoinfo / GlycanBuilder2