Skip to content

Add initial data extensions editor#2257

Closed
koesie10 wants to merge 32 commits intomainfrom
koesie10/data-extension-editor
Closed

Add initial data extensions editor#2257
koesie10 wants to merge 32 commits intomainfrom
koesie10/data-extension-editor

Conversation

@koesie10
Copy link
Copy Markdown
Member

@koesie10 koesie10 commented Apr 3, 2023

This is a big PR that adds the data extension editor as created during the hackathon into the main branch. It has been rebased on top of main and the most significant changes are:

The original branch is here.

In addition, I've also made some commits after the rebase:

  • The command is now only available behind the codeQL.dataExtensions.editor config setting. The user also needs to have codeQL.canary set.
  • I've renamed everything from "external API" to "data extensions editor". I've tried to separate out the changes as much as possible so that it's easy to see that they are purely renames.

I would highly recommend reviewing this commit-by-commit. I've ensured that each commit can be compiled by itself after the rebase, and each one only adds a small part.

My intention is to add tests and Storybook stories in a follow-up PR to ensure this PR doesn't get even larger than it already is.

Test plan

1-time setup

  1. Set the codeQL.canary and codeQL.dataExtensions.editor settings to true
  2. Copy the following query into the ql submodule at java/ql/src/Telemetry/FetchExternalAPIs.ql:
Query
/**
 * @name Usage of APIs coming from external libraries
 * @description A list of 3rd party APIs used in the codebase. Excludes test and generated code.
 * @tags telemetry
 * @id java/telemetry/fetch-external-apis
 */

import java
import semmle.code.java.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
import ExternalApi

private Call aUsage(ExternalApi api) {
  result.getCallee().getSourceDeclaration() = api and
  not result.getFile() instanceof GeneratedFile
}

private boolean isSupported(ExternalApi api) {
  api.isSupported() and result = true
  or
  api = any(FlowSummaryImpl::Public::NeutralCallable nsc).asCallable() and result = true
  or
  not api.isSupported() and
  not api = any(FlowSummaryImpl::Public::NeutralCallable nsc).asCallable() and
  result = false
}

from ExternalApi api, string apiName, boolean supported, Call usage
where
  apiName = api.getApiName() and
  supported = isSupported(api) and
  usage = aUsage(api)
select apiName, supported, usage

Testing the data extension editor

  1. Open the extension in the vscode-codeql-starter workspace
  2. Download the dsp-testing/sql2o-example database from GitHub and wait for it to download
  3. Select the dsp-testing/sql2o-example database (this should have been done automatically)
  4. Run the CodeQL: Open Data Extensions Editor command from the command palette
  5. It should show a few external API calls in this example
  6. Set org.sql2o.Connection#createQuery(String) to be a sink with input Argument[0] and kind sql
  7. Click Apply, it should now show as supported
  8. Click Download and generate. Enter the database dsp-testing/sql2o-import
  9. After the generation is finished, it should have modelled all the external API calls to sql2o
  10. Click Apply, it should now show sql2o calls as supported
  11. Close the data extensions editor and re-open it from the command palette. It should still show all sql2o calls as supported

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

elenatanasoiu and others added 30 commits April 3, 2023 12:26
This will change the "Show External Api Results" to only be available
behind the `codeQL.dataExtensions.editor` config flag. The user will
also need to have the `codeQL.canary` flag enabled.
This better covers what this module is intended to cover.
@koesie10 koesie10 added the secexp label Apr 3, 2023
@koesie10 koesie10 marked this pull request as ready for review April 3, 2023 13:28
@koesie10 koesie10 requested a review from a team as a code owner April 3, 2023 13:28
@koesie10 koesie10 requested a review from a team April 3, 2023 14:31
@charisk
Copy link
Copy Markdown
Contributor

charisk commented Apr 4, 2023

Thanks for taking the hackathon code and making it real!

This is a big PR. I appreciate you wanted to keep the origins and show the story but I personally find it hard to review such PRs. Is there an easy/sensible way to break it down into a handful of PRs instead?

@koesie10 koesie10 marked this pull request as draft April 4, 2023 10:00
@koesie10
Copy link
Copy Markdown
Member Author

koesie10 commented Apr 4, 2023

This is a big PR. I appreciate you wanted to keep the origins and show the story but I personally find it hard to review such PRs. Is there an easy/sensible way to break it down into a handful of PRs instead?

I've split this up into multiple PRs:

I'm closing this since all changes are now available in those other PRs.

@koesie10 koesie10 closed this Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants