feat: moving context command under config namespace

[Souvikns]

🚧 Work in progress, Not creating a draft because I need some opinion on the code.

Firstly my aim in this PR is to bring the context command under the config namespace so that the final command would look something like this asyncapi config context current also incorporating the updated ContextService class from #72.

Doing so I was experimenting with some things, I want an opinion from @derberg and @magicmatatjahu and @jotamusik if I am overcomplicating things.

File structure

I was trying something like this -

commands
│
└───config
│   │   router.tsx
│   │   component.tsx
│   │   message.tsx
│   │
│   └───context
│       │   router.tsx
│   
└───validate
    │   router.tsx

because the way our router is written, allows us to break the complete command string into commands, subcommands and arguments and we repeatedly use this to create hierarchical relations accordingly. Consider asyncapi config context list which would be something like this.

asyncapi
│
└────config
|      |
|      └──context
|         |
|         └──list
|         └──add
|
└────validate

Was trying to model this and create the file structure.

Injecting the context service and separating the logic for messages

Currently, we have a messages.ts file storing all the messages constants. I think that in the future all the different commands will have specific messages that would not be shared with any other commands. All commands have different input needs as well for that I was separating the logic and injecting it. I am trying something like this.

export interface IContextMessageWriter {
  list: FunctionComponent<{store?: {[name: string]: string}}>
}

@injectable()
export class ContextMessageWriter implements IContextMessageWriter {
  list: FunctionComponent<{store?: {[name: string]: string}}> = ({store}) => {
    return <Box flexDirection="column">
      {Object.keys(store as any).map((contextName: string) => <Text key={contextName}>{contextName}</Text>)}
    </Box>;
  }
}

All separate commands have separate messageWriters and then we can also reuse them for testing, Now I was not creating success and error messages because maybe after #76 we don't need to account for this. For now, I will write it.

And for the ContextService instead of creating hooks I was thinking of making something like this, though I don't know it is bad.

import { FunctionComponent } from 'react';
import { ContextService } from '../../../config/context';
import { inject, injectable } from 'tsyringe';
import {ContextMessageWriter, IContextMessageWriter} from './messages';

@injectable()
export class ContextComponent {
  constructor(
    private contextService: ContextService,
    @inject(ContextMessageWriter) private messageWriter: IContextMessageWriter
  ) {
  }
  list: FunctionComponent = () => {
    return this.messageWriter.list({store: this.contextService.context?.store});
  }
}

and then resolving container in the router.tsx doing something like this

import React from 'react';
import {container} from 'tsyringe';
import {CliInput} from '../../../CliModels';
import {ContextComponent} from './component';

const context = container.resolve(ContextComponent);

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const commandDictionary = (_cliInput: CliInput): Record<string, any> => ({
  list: <context.list />
});

export const contextRouter = (cliInput: CliInput): any => {
  const subCommand = CliInput.createSubCommand(cliInput);
  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
  return commandDictionary(subCommand)[subCommand.command!];
};

Related issue(s)

see also #37

[Souvikns]

@magicmatatjahu can you please help me with this.

The save function is not working as intended, I am trying everything but I can't figure out what is happening.

save(context: Context) {
    try {
      fs.writeFileSync(this.contextFilePath, JSON.stringify({
        current: context.current,
        store: context.store
      }), { encoding: 'utf8' });
      fs.writeFileSync(this.contextFilePath, JSON.stringify(context)); // changed this because initial was also not working
      return context;
    } catch (error) {
      console.warn(error);// eslint-disable-line no-undef, no-console
      return undefined;
    }
  }

I tried with context rather than breaking it because that was also not working.

whatever the context is only this is getting saved {"store":{}} and when I try to add a console statement to see what value I am receiving as the parameter I get this -

[magicmatatjahu]

@Souvikns As I tested, you save context three times and only last content is saved - I don't know why this function is executed so many times. Check the path from main function to save function, maybe you have some loop.

I have similar logs:

node ./dist/cli.js config context add spec ./spec.yaml

Context {
  current: undefined,
  store: {
    'test-spec': '/Users/matatjahu/Documents/AsyncAPI/cli/spec.yaml',
    'test2-spec': '/Users/matatjahu/Documents/AsyncAPI/cli/lol.yaml',
    spec: './spec.yaml'
  }
}
Context {
  current: 'spec',
  store: {
    'test-spec': '/Users/matatjahu/Documents/AsyncAPI/cli/spec.yaml',
    'test2-spec': '/Users/matatjahu/Documents/AsyncAPI/cli/lol.yaml',
    spec: './spec.yaml'
  }
}
Context {
  store: {
    'test-spec': '/Users/matatjahu/Documents/AsyncAPI/cli/spec.yaml',
    'test2-spec': '/Users/matatjahu/Documents/AsyncAPI/cli/lol.yaml'
  }
}

And only last content is saved, so please return to solution with:

      fs.writeFileSync(this.contextFilePath, JSON.stringify({
        current: context.current,
        store: context.store
      }), { encoding: 'utf8' });

and fix the "bug" with multiple savings.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[jotamusik]

I don't see think that this is a good approach. For me, this class has so many responsabilities and does not look like an intuitive react code.

It's managing so many things, validating input, throwing errors and managing the view. For me, all of this, should be individual components that call the hooks that manage those actions. 🤔

[Souvikns]

look like an intuitive react code.

😄 Yeah, I agree, Just trying to find a way to test, because calling the container.resolve function in the component itself makes the code coupled. With this, I am able to pass in mock context to test the implementation.

It's managing so many things, validating input, throwing errors and managing the view

Thinking on this issue. 🤔

[jotamusik]

If the key here is to be able to put context command inside the config one, why don't create just another config command router and route to the context one instead of all these changes?? Am I forgetting something that should be take in care? 🤔

[jotamusik]

Why do we need this ContextMessageWritter?? I think that if we want to abstract or name any of the views, we can create a separate component but for me, this isn't so intuitive 🤔

[Souvikns]

ContextMessageWritter might be overcomplicating things. Currently, I am just using it for running tests.

[jotamusik]

I understand the key of having the text strings that will be displayed, not this case as far as I know (even could be intestesting to take care of future internacionalization) but extracting all of these set of texts in order to make easier the asserts on the tests, is not a good practice. You could extract individual views as components in order to make unit testing and make sure that those individual views behave as you expect. This could be useful if you only want to assert (on an integration test or bigger unit test that contains any of these single compontents) that it contains certain string, not to do an assert about all the view tree. 😛